Skip to content

Commit c77d3a5

Browse files
authored
Merge branch 'main' into dependabot/npm_and_yarn/playwright/multi-893bcbaee7
2 parents 1b1216d + 24b2ef6 commit c77d3a5

File tree

7 files changed

+195
-41
lines changed

7 files changed

+195
-41
lines changed

src/app/content/highlights/components/Card.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ export interface CardProps {
5353
highlightOffsets?: { top: number; bottom: number };
5454
onHeightChange: (ref: React.RefObject<HTMLElement>) => void;
5555
isHidden: boolean;
56+
onBlurOptional?: () => void;
5657
}
5758

5859
type CardPropsWithBookAndPage = Omit<CardProps, 'book' | 'page'> & {
@@ -81,7 +82,10 @@ function useComputedProps(props: CardProps) {
8182
className: props.className,
8283
highlight: props.highlight,
8384
isActive: props.isActive,
84-
onBlur: props.blur,
85+
onBlur: /* istanbul ignore next */ () => {
86+
props.blur();
87+
if (props.onBlurOptional) props.onBlurOptional();
88+
},
8589
onHeightChange: props.onHeightChange,
8690
ref: element,
8791
shouldFocusCard: props.shouldFocusCard,

src/app/content/highlights/components/CardWrapper.spec.tsx

Lines changed: 83 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ const dispatchFocusOutEvent = (
2828
};
2929

3030
const dispatchHighlightToggle = (target: HTMLElement | undefined) => {
31-
dispatchKeyDownEvent({code: highlightKeyCombination.code, altKey: highlightKeyCombination.altKey, target});
31+
dispatchKeyDownEvent({ code: highlightKeyCombination.code, altKey: highlightKeyCombination.altKey, target });
3232
};
3333

3434
jest.mock('./Card', () => (props: any) => <span data-mock-card {...props} />);
@@ -297,52 +297,102 @@ describe('CardWrapper', () => {
297297
});
298298
});
299299

300+
it('handles useKeyCombination - hide/unhide cards', () => {
301+
const document = assertDocument();
302+
const highlight1 = createMockHighlight('id1');
303+
const highlight2 = createMockHighlight('id2');
304+
const highlightElement1 = document.createElement('span');
305+
const highlightElement2 = document.createElement('span');
306+
container.appendChild(highlightElement1);
307+
container.appendChild(highlightElement2);
308+
const component = renderer.create(
309+
<Provider store={store}>
310+
<CardWrapper
311+
container={container}
312+
highlights={[highlight1, highlight2]}
313+
/>
314+
</Provider>
315+
);
316+
317+
const cards = component.root.findAllByType(Card);
318+
319+
// Expect cards to be hidden
320+
renderer.act(() => {
321+
// Simulate pressing Escape to hide card
322+
dispatchKeyDownEvent({
323+
key: 'Escape',
324+
target: highlightElement1,
325+
});
326+
});
327+
328+
// Expect cards to be visible again
329+
renderer.act(() => {
330+
// Simulate pressing Enter to unhide cards
331+
dispatchKeyDownEvent({
332+
key: 'Enter',
333+
target: highlightElement1,
334+
});
335+
});
336+
337+
// Expect all cards to be visible
338+
renderer.act(() => {
339+
// Simulate pressing Tab to unhide all cards
340+
dispatchKeyDownEvent({
341+
key: 'Tab',
342+
target: highlightElement1,
343+
});
344+
});
345+
346+
expect(cards[0].props.isHidden).toBe(false);
347+
expect(cards[1].props.isHidden).toBe(false);
348+
});
349+
300350
it(
301351
'handles useKeyCombination - noop if trigerred in element that we dont support '
302352
+ 'or with another key combination',
303353
() => {
304-
const document = assertDocument();
305-
const highlight = createMockHighlight();
306-
const highlightElement = document.createElement('span');
307-
container.appendChild(highlightElement);
354+
const document = assertDocument();
355+
const highlight = createMockHighlight();
356+
const highlightElement = document.createElement('span');
357+
container.appendChild(highlightElement);
308358

309-
const textarea = document.createElement('textarea');
310-
container.appendChild(textarea);
359+
const textarea = document.createElement('textarea');
360+
container.appendChild(textarea);
311361

312-
const elementInsideContainer = document.createElement('div');
313-
container.appendChild(elementInsideContainer);
362+
const elementInsideContainer = document.createElement('div');
363+
container.appendChild(elementInsideContainer);
314364

315-
const elementOutsideOfTheContainer = document.createElement('div');
316-
document.body.appendChild(elementOutsideOfTheContainer);
365+
const elementOutsideOfTheContainer = document.createElement('div');
366+
document.body.appendChild(elementOutsideOfTheContainer);
317367

318-
const cardWrapperElement = document.createElement('div');
368+
const cardWrapperElement = document.createElement('div');
319369

320-
store.dispatch(focusHighlight(highlight.id));
370+
store.dispatch(focusHighlight(highlight.id));
321371

322-
const component = renderer.create(<Provider store={store}>
323-
<CardWrapper container={container} highlights={[highlight]} />
324-
</Provider>, { createNodeMock: () => cardWrapperElement });
372+
const component = renderer.create(<Provider store={store}>
373+
<CardWrapper container={container} highlights={[highlight]} />
374+
</Provider>, { createNodeMock: () => cardWrapperElement });
325375

326-
renderer.act(() => {
327-
const card = component.root.findByType(Card);
328-
expect(card.props.shouldFocusCard).toEqual(false);
329-
});
376+
renderer.act(() => {
377+
const card = component.root.findByType(Card);
378+
expect(card.props.shouldFocusCard).toEqual(false);
379+
});
330380

331-
renderer.act(() => {
332-
dispatchHighlightToggle(textarea);
381+
renderer.act(() => {
382+
dispatchHighlightToggle(textarea);
333383

334-
dispatchHighlightToggle(elementOutsideOfTheContainer);
384+
dispatchHighlightToggle(elementOutsideOfTheContainer);
335385

336-
dispatchKeyDownEvent({key: 'anotherkeythatwedontsupport', target: elementInsideContainer});
337-
});
386+
dispatchKeyDownEvent({ key: 'anotherkeythatwedontsupport', target: elementInsideContainer });
387+
});
338388

339-
renderer.act(() => {
340-
const card = component.root.findByType(Card);
341-
expect(card.props.shouldFocusCard).toEqual(false);
342-
});
389+
renderer.act(() => {
390+
const card = component.root.findByType(Card);
391+
expect(card.props.shouldFocusCard).toEqual(false);
392+
});
343393

344-
expect(highlight.focus).not.toHaveBeenCalled();
345-
});
394+
expect(highlight.focus).not.toHaveBeenCalled();
395+
});
346396

347397
it('handles useKeyCombination - noop if focusedHighlight is undefined', () => {
348398
const document = assertDocument();
@@ -417,7 +467,7 @@ describe('CardWrapper', () => {
417467
expect(store.getState().content.highlights.currentPage.focused).toEqual(highlight.id);
418468

419469
renderer.act(() => {
420-
dispatchKeyDownEvent({key: highlightKeyCombination.key!, target: cardElement});
470+
dispatchKeyDownEvent({ key: highlightKeyCombination.key!, target: cardElement });
421471
});
422472

423473
expect(highlight.focus).not.toHaveBeenCalled();
@@ -427,7 +477,7 @@ describe('CardWrapper', () => {
427477
it('loads', () => {
428478
renderer.create(<Provider store={store}>
429479
<CardWrapper container={container} highlights={[]} />
430-
</Provider>);
480+
</Provider>);
431481

432482
runHooks(renderer);
433483

src/app/content/highlights/components/CardWrapper.tsx

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { highlightKeyCombination } from '../constants';
1414
import { focused } from '../selectors';
1515
import Card from './Card';
1616
import { mainWrapperStyles } from './cardStyles';
17-
import { getHighlightOffset, noopKeyCombinationHandler, updateCardsPositions } from './cardUtils';
17+
import { editCardVisibilityHandler, getHighlightOffset, noopKeyCombinationHandler, updateCardsPositions } from './cardUtils';
1818

1919
export interface WrapperProps {
2020
hasQuery: boolean;
@@ -37,6 +37,10 @@ const Wrapper = ({highlights, className, container, highlighter}: WrapperProps)
3737
() => highlights.find((highlight) => highlight.id === focusedId),
3838
[focusedId, highlights]);
3939
const setNewCardsPositionsRef = React.useRef<() => void>();
40+
const [isHiddenByEscape, dispatch] = React.useReducer(
41+
editCardVisibilityHandler,
42+
new Map(highlights.map((highlight) => [highlight.id, false]))
43+
);
4044

4145
// This function is triggered by keyboard shortcut defined in useKeyCombination(...)
4246
// It moves focus between Card component and highlight in the content.
@@ -54,10 +58,32 @@ const Wrapper = ({highlights, className, container, highlighter}: WrapperProps)
5458
}
5559
}, [focusedHighlight, focusedId]);
5660

61+
const hideCard = () => {
62+
dispatch({ type: 'HIDE', id: focusedId });
63+
};
64+
65+
const showCard = () => {
66+
dispatch({ type: 'SHOW', id: focusedId });
67+
};
68+
69+
const showAllCards = () => {
70+
dispatch({ type: 'SHOW_ALL' });
71+
};
72+
5773
useKeyCombination(highlightKeyCombination, moveFocus, noopKeyCombinationHandler([container, element]));
5874

59-
// Allow to move back to highlight from EditCard using Escape key
60-
useKeyCombination({key: 'Escape'}, moveFocus);
75+
/*
76+
* Allow to show EditCard using Enter key
77+
* It is important to preserve the default behavior of Enter key
78+
*/
79+
useKeyCombination({key: 'Enter'}, showCard, undefined, false);
80+
81+
// Allow to hide EditCard using Escape key
82+
useKeyCombination({key: 'Escape'}, hideCard, undefined, false);
83+
84+
// After move focus, reset visibility of all cards
85+
useKeyCombination({ key: 'Tab' }, showAllCards, undefined, false);
86+
useKeyCombination({ key: 'Tab', shiftKey: true }, showAllCards, undefined, false);
6187

6288
// Clear shouldFocusCard when focus is lost from the CardWrapper.
6389
// If we don't do this then card related for the focused highlight will be focused automatically.
@@ -128,7 +154,8 @@ const Wrapper = ({highlights, className, container, highlighter}: WrapperProps)
128154
onHeightChange={(ref: React.RefObject<HTMLElement>) => onHeightChange(highlight.id, ref)}
129155
zIndex={highlights.length - index}
130156
shouldFocusCard={focusThisCard}
131-
isHidden={checkIfHiddenByCollapsedAncestor(highlight)}
157+
isHidden={checkIfHiddenByCollapsedAncestor(highlight) || isHiddenByEscape.get(highlight.id)}
158+
onBlurOptional={showAllCards}
132159
/>;
133160
})}
134161
</div>

src/app/content/highlights/components/__snapshots__/CardWrapper.spec.tsx.snap

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ exports[`CardWrapper matches snapshot 1`] = `
6161
}
6262
}
6363
isHidden={false}
64+
onBlurOptional={[Function]}
6465
onHeightChange={[Function]}
6566
shouldFocusCard={false}
6667
zIndex={1}

src/app/content/highlights/components/cardUtils.spec.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { Highlight } from '@openstax/highlighter';
22
import { HighlightColorEnum } from '@openstax/highlighter/dist/api';
33
import createMockHighlight from '../../../../test/mocks/highlight';
44
import { assertDocument } from '../../../utils';
5-
import { generateUpdatePayload, getHighlightTopOffset } from './cardUtils';
5+
import { generateUpdatePayload, getHighlightTopOffset, editCardVisibilityHandler } from './cardUtils';
66

77
describe('cardUtils', () => {
88
it('returns undefined if passed container is undefined', () => {
@@ -48,3 +48,50 @@ describe('generateUpdatePayload', () => {
4848
expect(payload.updatePayload.highlight).toEqual({...update, color: oldData.color});
4949
});
5050
});
51+
52+
describe('editCardVisibilityHandler', () => {
53+
const highlights = [
54+
{id: 'highlight1', annotation: 'annotation1'},
55+
{id: 'highlight2', annotation: 'annotation2'},
56+
{id: 'highlight3', annotation: 'annotation3'},
57+
];
58+
const state = new Map(highlights.map((highlight) => [highlight.id, false]));
59+
60+
it('hide highlight', () => {
61+
const result = editCardVisibilityHandler(state, { type: 'HIDE', id: 'highlight1'});
62+
expect(result.get('highlight1')).toBe(true);
63+
expect(result.get('highlight2')).toBe(false);
64+
expect(result.get('highlight3')).toBe(false);
65+
});
66+
67+
it('hide highlight - unknown id', () => {
68+
const result = editCardVisibilityHandler(state, { type: 'HIDE'});
69+
expect(result.get('highlight1')).toBe(false);
70+
});
71+
72+
it('show highlight', () => {
73+
const result = editCardVisibilityHandler(state, { type: 'SHOW', id: 'highlight1'});
74+
expect(result.get('highlight1')).toBe(false);
75+
expect(result.get('highlight2')).toBe(false);
76+
expect(result.get('highlight3')).toBe(false);
77+
});
78+
79+
it('show highlight - unknown id', () => {
80+
const result = editCardVisibilityHandler(state, { type: 'SHOW'});
81+
expect(result.get('highlight1')).toBe(false);
82+
});
83+
84+
it('show all highlights', () => {
85+
const firstResult = editCardVisibilityHandler(state, { type: 'HIDE', id: 'highlight1'});
86+
expect(firstResult.get('highlight1')).toBe(true);
87+
const secondResult = editCardVisibilityHandler(state, { type: 'SHOW_ALL'});
88+
expect(secondResult.get('highlight1')).toBe(false);
89+
expect(secondResult.get('highlight2')).toBe(false);
90+
expect(secondResult.get('highlight3')).toBe(false);
91+
});
92+
93+
it('return state when action type does not exist', () => {
94+
const result = editCardVisibilityHandler(state, { type: 'SHOW_ANY'});
95+
expect(result.get('highlight1')).toBe(false);
96+
});
97+
});

src/app/content/highlights/components/cardUtils.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,3 +215,27 @@ export const noopKeyCombinationHandler = (
215215
if (activeElement.nodeName === 'TEXTAREA') { return true; }
216216
return false;
217217
};
218+
219+
export const editCardVisibilityHandler = (state: Map<string, boolean>, action: { type: string; id?: string }) => {
220+
switch (action.type) {
221+
case 'SHOW': {
222+
if (!action.id) return state;
223+
const newState = new Map(state);
224+
newState.set(action.id, false);
225+
return newState;
226+
}
227+
case 'HIDE': {
228+
if (!action.id) return state;
229+
const updatedState = new Map(state);
230+
updatedState.set(action.id, true);
231+
return updatedState;
232+
}
233+
case 'SHOW_ALL': {
234+
const allVisibleState = new Map(state);
235+
allVisibleState.forEach((_, key) => allVisibleState.set(key, false));
236+
return allVisibleState;
237+
}
238+
default:
239+
return state;
240+
}
241+
};

src/app/reactUtils.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,8 @@ export const keyboardEventMatchesCombination = (options: KeyCombinationOptions,
449449
export const useKeyCombination = (
450450
options: KeyCombinationOptions,
451451
callback: (event: KeyboardEvent) => void,
452-
noopHandler?: (activeElement: Element) => boolean
452+
noopHandler?: (activeElement: Element) => boolean,
453+
preventDefault = true
453454
) => {
454455
const document = assertDocument();
455456

@@ -458,7 +459,7 @@ export const useKeyCombination = (
458459
return;
459460
}
460461
if (keyboardEventMatchesCombination(options, event)) {
461-
event.preventDefault();
462+
if (preventDefault) event.preventDefault();
462463
callback(event);
463464
}
464465
// eslint-disable-next-line react-hooks/exhaustive-deps

0 commit comments

Comments
 (0)