Skip to content

Commit 70a942a

Browse files
DananjiDananji
committed
Fixes to event handlers from manual testing, add unit tests for new funcationaly
1 parent 1f4529e commit 70a942a

File tree

6 files changed

+508
-38
lines changed

6 files changed

+508
-38
lines changed

‎src/components/MarkersDisplay/Annotations/AnnotationRow.js

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,17 @@ const AnnotationRow = ({
157157
setIsShowMoreRef(!isShowMoreRef.current);
158158
};
159159

160+
/**
161+
* Keydown event handler for show more/less button in the annotation text
162+
* @param {Event} e keydown event
163+
*/
164+
const handleShowMoreLessKeydown = (e) => {
165+
if (e.key == 'Enter' || e.key == ' ') {
166+
e.preventDefault();
167+
handleShowMoreLessClick();
168+
}
169+
};
170+
160171
/**
161172
* Click event handler for show/hide overflowing tags button for
162173
* each annotation row.
@@ -176,7 +187,7 @@ const AnnotationRow = ({
176187
* button for 'Space' (32) and 'Enter' (13) keys.
177188
*/
178189
const handleShowMoreTagsKeyDown = (e) => {
179-
if (e.keyCode === 32 || e.keyCode === 13) {
190+
if (e.key === ' ' || e.key === 'Enter') {
180191
e.preventDefault();
181192
handleShowMoreTagsClicks();
182193
}
@@ -191,28 +202,45 @@ const AnnotationRow = ({
191202
* @returns
192203
*/
193204
const handleKeyDown = (e) => {
205+
// Get links/buttons inside the annotation row
206+
const linksAndButtons = annotationRef.current.querySelectorAll('button, a');
207+
if (linksAndButtons?.length > 0) {
208+
for (let i = 0; i < linksAndButtons.length; i++) {
209+
linksAndButtons[i].tabIndex = -1;
210+
}
211+
}
194212
const handleTab = (e) => {
195-
// Get links/buttons inside the annotation row
196-
const linksAndButtons = annotationRef.current.querySelectorAll('button, a');
213+
let nextIndex = focusedIndex.current;
197214
// Allow tabbing through links/buttons if they exist, and do nothing if not
198215
if (linksAndButtons?.length > 0) {
199216
if (e.shiftKey) {
200-
nextIndex = (focusedIndex.current - 1 + linksAndButtons.length) % linksAndButtons.length;
201-
// Stop the event from bubbling up to keydown event handler in parent element in AnnotationDisplay
202-
e.stopPropagation();
217+
// When focused on a link/button that is not the first in annotation trap
218+
// focus within the annotation else let the event bubble up to move focus
219+
// away from the focused annotation
220+
if (focusedIndex.current > 0) {
221+
nextIndex = (focusedIndex.current - 1) % linksAndButtons.length;
222+
// Stop the event from bubbling up to keydown event handler in parent element in AnnotationDisplay
223+
e.stopPropagation();
224+
}
203225
// Prevent default behavior. Focus shifts to the link/button prior to the one at nextIndex without this
204226
e.preventDefault();
205227
} else {
206228
nextIndex = (focusedIndex.current + 1) % linksAndButtons.length;
229+
e.preventDefault();
207230
}
208-
if (nextIndex != focusedIndex.current && linksAndButtons?.length > 0) {
231+
232+
// Focus on link/button if it exists
233+
if (linksAndButtons[nextIndex]) {
209234
linksAndButtons[nextIndex].focus();
210235
setFocusedIndex(nextIndex);
211236
}
237+
} else {
238+
// Stop default behavior when there are no links/buttons in the annotation
239+
e.preventDefault();
240+
return;
212241
}
213242
};
214243

215-
let nextIndex = focusedIndex.current;
216244
if (e.key === 'Enter' || e.key === ' ') {
217245
handleOnClick(e);
218246
}
@@ -232,44 +260,44 @@ const AnnotationRow = ({
232260
ref={annotationRef}
233261
onClick={handleOnClick}
234262
onKeyDown={handleKeyDown}
235-
data-testid="annotation-row"
263+
data-testid='annotation-row'
236264
className={cx(
237-
"ramp--annotations__annotation-row",
265+
'ramp--annotations__annotation-row',
238266
isActive && 'active'
239267
)}
240268
>
241-
<div key={`row_${index}`} className="ramp--annotations__annotation-row-time-tags">
269+
<div key={`row_${index}`} className='ramp--annotations__annotation-row-time-tags'>
242270
<div
243271
key={`times_${index}`}
244-
className="ramp--annotations__annotation-times"
272+
className='ramp--annotations__annotation-times'
245273
ref={annotationTimesRef}
246274
>
247275
{time?.start != undefined && (
248276
<span
249-
className="ramp--annotations__annotation-start-time"
250-
data-testid="annotation-start-time">
277+
className='ramp--annotations__annotation-start-time'
278+
data-testid='annotation-start-time'>
251279
{timeToHHmmss(time?.start, true, true)}
252280
</span>
253281
)}
254282
{time?.end != undefined && (
255283
<span
256-
className="ramp--annotations__annotation-end-time"
257-
data-testid="annotation-end-time">
284+
className='ramp--annotations__annotation-end-time'
285+
data-testid='annotation-end-time'>
258286
{` - ${timeToHHmmss(time?.end, true, true)}`}
259287
</span>
260288
)}
261289
</div>
262290
<div
263291
key={`tags_${index}`}
264-
className="ramp--annotations__annotation-tags"
292+
className='ramp--annotations__annotation-tags'
265293
data-testid={`annotation-tags-${index}`}
266294
ref={annotationTagsRef}
267295
>
268296
{tags?.length > 0 && tags.map((tag, i) => {
269297
return (
270298
<p
271299
key={`tag_${i}`}
272-
className="ramp--annotations__annotation-tag"
300+
className='ramp--annotations__annotation-tag'
273301
data-testid={`annotation-tag-${i}`}
274302
style={{ backgroundColor: tag.tagColor }}
275303
>
@@ -283,7 +311,7 @@ const AnnotationRow = ({
283311
role='button'
284312
aria-label={showMoreTags ? 'Show hidden tags' : 'Hide overflowing tags'}
285313
aria-pressed={showMoreTags ? 'false' : 'true'}
286-
className="ramp--annotations__show-more-tags"
314+
className='ramp--annotations__show-more-tags'
287315
data-testid={`show-more-annotation-tags-${index}`}
288316
onClick={handleShowMoreTagsClicks}
289317
onKeyDown={handleShowMoreTagsKeyDown}
@@ -296,14 +324,14 @@ const AnnotationRow = ({
296324
</div>
297325
<div
298326
key={`text_${index}`}
299-
className="ramp--annotations__annotation-texts"
327+
className='ramp--annotations__annotation-texts'
300328
ref={annotationTextsRef}
301329
>
302330
{textToShow?.length > 0 && (
303331
<p
304332
key={`text_${index}`}
305333
data-testid={`annotation-text-${index}`}
306-
className="ramp--annotations__annotation-text"
334+
className='ramp--annotations__annotation-text'
307335
dangerouslySetInnerHTML={{ __html: textToShow }}
308336
></p>
309337
)}
@@ -313,9 +341,10 @@ const AnnotationRow = ({
313341
role='button'
314342
aria-label={isShowMoreRef.current ? 'show more' : 'show less'}
315343
aria-pressed={isShowMoreRef.current ? 'false' : 'true'}
316-
className="ramp--annotations__show-more-less"
344+
className='ramp--annotations__show-more-less'
317345
data-testid={`annotation-show-more-${index}`}
318346
onClick={handleShowMoreLessClick}
347+
onKeyDown={handleShowMoreLessKeydown}
319348
>
320349
{isShowMoreRef.current ? 'Show more' : 'Show less'}
321350
</button>)

‎src/components/MarkersDisplay/Annotations/AnnotationRow.test.js

Lines changed: 186 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -907,4 +907,190 @@ describe('AnnotationRow component', () => {
907907
expect(screen.queryByText('Show more')).not.toBeInTheDocument();
908908
});
909909
});
910+
911+
describe('allows keyboard navigation', () => {
912+
describe('within a plain annotation text', () => {
913+
const annotation = {
914+
id: 'http://example.com/manifest/canvas/1/annotation-page/1/annotation/1',
915+
canvasId: 'http://example.com/manifest/canvas/1',
916+
motivation: ['supplementing'],
917+
time: { start: 0, end: 10 },
918+
value: [
919+
{
920+
format: 'text/plain',
921+
purpose: ['supplementing'],
922+
value: `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua.`
923+
},
924+
{
925+
format: 'text/plain',
926+
purpose: ['supplementing'],
927+
value: `Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat.`
928+
},
929+
]
930+
};
931+
932+
beforeEach(() => {
933+
// Mock Canvas, getComputedStyle, and clientWidth of annotationTextRef for a controlled test
934+
jest.spyOn(window, 'getComputedStyle').mockImplementation((ele) => ({
935+
lineHeight: '24px',
936+
fontSize: '16px',
937+
font: '16px / 24px "Open Sans", sans-serif',
938+
}));
939+
Object.defineProperty(HTMLCanvasElement.prototype, 'getContext', {
940+
value: jest.fn(() => ({
941+
measureText: jest.fn((texts) => ({ width: texts.length * 10 })),
942+
})),
943+
});
944+
Object.defineProperty(HTMLElement.prototype, 'clientWidth', {
945+
configurable: true,
946+
get: jest.fn(() => 800),
947+
});
948+
949+
render(<AnnotationRow
950+
{...props}
951+
showMoreSettings={{ enableShowMore: true, textLineLimit: 6 }}
952+
annotation={annotation}
953+
/>);
954+
});
955+
956+
test('renders successfully', () => {
957+
expect(screen.getByTestId('annotation-row')).toBeInTheDocument();
958+
expect(screen.queryAllByTestId('annotation-tag-0').length).toBe(0);
959+
960+
expect(screen.getByTestId('annotation-start-time')).toHaveTextContent('00:00:00.000');
961+
expect(screen.queryByTestId('annotation-end-time')).toHaveTextContent('00:00:10.000');
962+
963+
expect(screen.queryByTestId('annotation-text-0')).toBeInTheDocument();
964+
expect(screen.getByTestId('annotation-text-0').textContent.length).toBeGreaterThan(0);
965+
});
966+
967+
test('activates the annotation on Enter keypress when focused', () => {
968+
screen.getByTestId('annotation-row').focus();
969+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Enter', keyCode: 13 });
970+
971+
expect(checkCanvasMock).toHaveBeenCalled();
972+
});
973+
974+
test('activates the annotation on Space keypress when focused', () => {
975+
screen.getByTestId('annotation-row').focus();
976+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: ' ', keyCode: 32 });
977+
978+
expect(checkCanvasMock).toHaveBeenCalledTimes(1);
979+
});
980+
981+
test('does nothing on Tab keypress', () => {
982+
screen.getByTestId('annotation-row').focus();
983+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Tab', keyCode: 9 });
984+
985+
expect(screen.getByTestId('annotation-row')).toHaveFocus();
986+
});
987+
});
988+
989+
describe('within an annotation text with links', () => {
990+
const annotation = {
991+
id: 'http://example.com/manifest/canvas/1/annotation-page/1/annotation/1',
992+
canvasId: 'http://example.com/manifest/canvas/1',
993+
motivation: ['supplementing'],
994+
time: { start: 0, end: 10 },
995+
value: [
996+
{
997+
format: 'text/plain',
998+
purpose: ['supplementing'],
999+
value: `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. <sup><a href="https://example1.com">t</a></sup>Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.`
1000+
},
1001+
{
1002+
format: 'text/plain',
1003+
purpose: ['supplementing'],
1004+
value: `Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis<sup><a href="https://example2.com">t</a></sup> nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.`
1005+
},
1006+
{
1007+
format: 'text/plain',
1008+
purpose: ['supplementing'],
1009+
value: `Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in <sup><a href="https://example3.com">t</a></sup>reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur.`
1010+
},
1011+
]
1012+
};
1013+
1014+
beforeEach(() => {
1015+
// Mock Canvas, getComputedStyle, and clientWidth of annotationTextRef for a controlled test
1016+
jest.spyOn(window, 'getComputedStyle').mockImplementation((ele) => ({
1017+
lineHeight: '24px',
1018+
fontSize: '16px',
1019+
font: '16px / 24px "Open Sans", sans-serif',
1020+
}));
1021+
Object.defineProperty(HTMLCanvasElement.prototype, 'getContext', {
1022+
value: jest.fn(() => ({
1023+
measureText: jest.fn((texts) => ({ width: texts.length * 10 })),
1024+
})),
1025+
});
1026+
Object.defineProperty(HTMLElement.prototype, 'clientWidth', {
1027+
configurable: true,
1028+
get: jest.fn(() => 800),
1029+
});
1030+
1031+
render(<AnnotationRow
1032+
{...props}
1033+
showMoreSettings={{ enableShowMore: true, textLineLimit: 6 }}
1034+
annotation={annotation}
1035+
/>);
1036+
});
1037+
1038+
test('renders successfully with links and \'Show more\' button', () => {
1039+
expect(screen.getByTestId('annotation-row')).toBeInTheDocument();
1040+
expect(screen.queryAllByTestId('annotation-tag-0').length).toBe(0);
1041+
1042+
expect(screen.getByTestId('annotation-start-time')).toHaveTextContent('00:00:00.000');
1043+
expect(screen.queryByTestId('annotation-end-time')).toHaveTextContent('00:00:10.000');
1044+
1045+
expect(screen.queryByTestId('annotation-text-0')).toBeInTheDocument();
1046+
expect(screen.getByTestId('annotation-text-0').textContent.length).toBeGreaterThan(0);
1047+
1048+
// Contains links and a show more button
1049+
expect(screen.getByTestId('annotation-text-0').querySelectorAll('a').length).toBeGreaterThan(0);
1050+
expect(screen.queryAllByTestId('annotation-show-more-0').length).toBe(1);
1051+
expect(screen.queryByText('Show more')).toBeInTheDocument();
1052+
1053+
});
1054+
1055+
test('activates the annotation on Enter keypress when focused', () => {
1056+
screen.getByTestId('annotation-row').focus();
1057+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Enter', keyCode: 13 });
1058+
1059+
expect(checkCanvasMock).toHaveBeenCalled();
1060+
});
1061+
1062+
test('activates the annotation on Space keypress when focused', () => {
1063+
screen.getByTestId('annotation-row').focus();
1064+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: ' ', keyCode: 32 });
1065+
1066+
expect(checkCanvasMock).toHaveBeenCalledTimes(1);
1067+
});
1068+
1069+
test('moves focus to first link on Tab keypress', () => {
1070+
screen.getByTestId('annotation-row').focus();
1071+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Tab', keyCode: 9 });
1072+
1073+
expect(screen.getByTestId('annotation-row')).not.toHaveFocus();
1074+
expect(screen.getByTestId('annotation-text-0').querySelectorAll('a')[0]).toHaveFocus();
1075+
});
1076+
1077+
test('moves focus to previous link on Shift+Tab keypress', () => {
1078+
screen.getByTestId('annotation-row').focus();
1079+
// Press Tab key twice to move focus to show more button
1080+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Tab', keyCode: 9 });
1081+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Tab', keyCode: 9 });
1082+
1083+
// Focus is moved to the 'Show more' button
1084+
expect(screen.getByTestId('annotation-row')).not.toHaveFocus();
1085+
expect(screen.getByText('Show more')).toHaveFocus();
1086+
1087+
// Press Shift+Tab key
1088+
fireEvent.keyDown(screen.getByTestId('annotation-row'), { key: 'Tab', keyCode: 9, shiftKey: true });
1089+
1090+
// Focus is moved to the link text
1091+
expect(screen.getByTestId('annotation-text-0').querySelectorAll('a')[0]).toHaveFocus();
1092+
expect(screen.getByText('Show more')).not.toHaveFocus();
1093+
});
1094+
});
1095+
});
9101096
});

0 commit comments

Comments
 (0)