Skip to content

Commit 540cd72

Browse files
DananjiDananji
committed
Code review: add additional keydown events, enable natural tab order at the end
1 parent 70a942a commit 540cd72

File tree

5 files changed

+197
-40
lines changed

5 files changed

+197
-40
lines changed

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

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -204,11 +204,6 @@ const AnnotationRow = ({
204204
const handleKeyDown = (e) => {
205205
// Get links/buttons inside the annotation row
206206
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-
}
212207
const handleTab = (e) => {
213208
let nextIndex = focusedIndex.current;
214209
// Allow tabbing through links/buttons if they exist, and do nothing if not
@@ -234,10 +229,6 @@ const AnnotationRow = ({
234229
linksAndButtons[nextIndex].focus();
235230
setFocusedIndex(nextIndex);
236231
}
237-
} else {
238-
// Stop default behavior when there are no links/buttons in the annotation
239-
e.preventDefault();
240-
return;
241232
}
242233
};
243234

@@ -316,6 +307,7 @@ const AnnotationRow = ({
316307
onClick={handleShowMoreTagsClicks}
317308
onKeyDown={handleShowMoreTagsKeyDown}
318309
ref={moreTagsButtonRef}
310+
tabIndex={-1}
319311
>
320312
<i className={`arrow ${showMoreTags ? 'right' : 'left'}`}></i>
321313
</button>
@@ -345,6 +337,7 @@ const AnnotationRow = ({
345337
data-testid={`annotation-show-more-${index}`}
346338
onClick={handleShowMoreLessClick}
347339
onKeyDown={handleShowMoreLessKeydown}
340+
tabIndex={-1}
348341
>
349342
{isShowMoreRef.current ? 'Show more' : 'Show less'}
350343
</button>)

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

Lines changed: 96 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -162,34 +162,58 @@ const AnnotationSetSelect = ({
162162
* @param {Event} e keydown event from dropdown button
163163
*/
164164
const handleDropdownKeyPress = (e) => {
165-
// Close the dropdown on 'Escape' keypress if it is open
166-
if (e.key === 'Escape') {
167-
e.preventDefault();
168-
if (isOpenRef.current) toggleDropdown();
169-
}
170-
171-
// Toggle dropdown on Enter/Space keypresses
172-
if (e.key === 'Enter' || e.key === ' ') {
173-
e.preventDefault();
174-
toggleDropdown();
175-
}
176-
// Open the dropdown and move focus to first option on ArrowDown/ArrowUp keypresses
177-
if (e.key === 'ArrowDown' || e.key === 'ArrowUp') {
178-
if (!isOpenRef.current) {
179-
setIsOpen(true);
180-
// Keep the container scrolled to top. Without this the first option
181-
// gets out of view when dropdown is programatically opened
182-
setTimeout(() => {
183-
if (dropDownRef.current) dropDownRef.current.scrollTop = 0;
184-
}, 0);
185-
}
186-
187-
// Move focus to the first option in the list
188-
const firstOption = document.querySelector(".annotations-dropdown-item");
189-
if (firstOption) {
190-
firstOption.focus();
191-
setCurrentIndex(0);
165+
const handleHomeEndKeys = () => {
166+
// If dropdown is open and pressed key is either Home or PageUp/End or PageDown
167+
if (isOpenRef.current && dropDownRef.current) {
168+
// Get all options in the dropdown
169+
const allOptions = dropDownRef.current.children;
170+
// Move focus to the first option in the list
171+
if ((e.key === 'Home' || e.key === 'PageUp') && allOptions?.length > 0) {
172+
allOptions[0].focus();
173+
setCurrentIndex(0);
174+
}
175+
// Move focus to the last option in the list
176+
if ((e.key === 'End' || e.key === 'PageDown') && allOptions?.length > 0) {
177+
allOptions[allOptions.length - 1].focus();
178+
setCurrentIndex(allOptions.length - 1);
179+
}
192180
}
181+
};
182+
switch (e.key) {
183+
case 'Enter':
184+
case ' ':
185+
e.preventDefault();
186+
toggleDropdown();
187+
break;
188+
case 'ArrowDown':
189+
case 'ArrowUp':
190+
if (!isOpenRef.current) setIsOpen(true);
191+
// Move focus to the first option in the list
192+
const firstOption = document.querySelector(".annotations-dropdown-item");
193+
if (firstOption) {
194+
e.preventDefault();
195+
firstOption.focus();
196+
setCurrentIndex(0);
197+
}
198+
break;
199+
case 'Home':
200+
case 'PageUp':
201+
case 'End':
202+
case 'PageDown':
203+
handleHomeEndKeys();
204+
break;
205+
case 'Escape':
206+
e.preventDefault();
207+
if (isOpenRef.current) toggleDropdown();
208+
break;
209+
default:
210+
// Do nothing if a combination key is pressed
211+
if (e.ctrlKey || e.altKey || e.metaKey || e.shiftKey || e.key.length > 1) {
212+
return;
213+
}
214+
if (!isOpenRef.current) setIsOpen(true);
215+
handlePrintableChars(e);
216+
break;
193217
}
194218
};
195219

@@ -222,6 +246,18 @@ const AnnotationSetSelect = ({
222246
// Move to previous option on ArrowUp keypress and wraps to last option when top is reached
223247
nextIndex = (currentIndex.current - 1 + allOptions.length) % allOptions.length;
224248
break;
249+
case 'Home':
250+
case 'PageUp':
251+
e.preventDefault();
252+
// Move to the first option the in the list
253+
nextIndex = 0;
254+
break;
255+
case 'End':
256+
case 'PageDown':
257+
e.preventDefault();
258+
// Move to the last option in the list
259+
nextIndex = allOptions.length - 1;
260+
break;
225261
case 'Escape':
226262
e.preventDefault();
227263
// Close the dropdown and move focus to dropdown button
@@ -232,15 +268,46 @@ const AnnotationSetSelect = ({
232268
// Close dropdown and move focus out to the next element in the DOM
233269
toggleDropdown();
234270
break;
271+
default:
272+
handlePrintableChars(e);
273+
break;
235274
}
236275

237-
// Update focus on the selected option using ArrowDown/ArrowUp keys
276+
// Focus option at nextIndex and scroll it into view
238277
if (nextIndex !== currentIndex.current) {
239278
allOptions[nextIndex].focus();
279+
allOptions[nextIndex].scrollIntoView();
240280
setCurrentIndex(nextIndex);
241281
}
242282
};
243283

284+
/**
285+
* When a printable character is pressed match it against the first character
286+
* of each option in the list and focus the first option with a match
287+
* @param {Event} e keydown event
288+
*/
289+
const handlePrintableChars = (e) => {
290+
const keyChar = e.key;
291+
const isPrintableChar = keyChar.length === 1 && keyChar.match(/\S/);
292+
setTimeout(() => {
293+
if (isPrintableChar && dropDownRef.current) {
294+
const allOptions = dropDownRef.current.children;
295+
// Ignore first option for select all when there are multiple options
296+
const ignoreSelectAll = allOptions.length > 1 ? true : false;
297+
for (let i in allOptions) {
298+
if (ignoreSelectAll && i == 0) {
299+
continue;
300+
}
301+
if (allOptions[i].textContent?.trim()[0].toLowerCase() === keyChar) {
302+
allOptions[i].focus();
303+
setCurrentIndex(i);
304+
break;
305+
}
306+
}
307+
}
308+
}, 0);
309+
};
310+
244311
/**
245312
* Handle keydown event for the checkbox for turning auto-scroll on/off
246313
* @param {Event} e keydown event
@@ -265,7 +332,7 @@ const AnnotationSetSelect = ({
265332
className='ramp--annotations__multi-select-header'
266333
onClick={toggleDropdown}
267334
onKeyDown={handleDropdownKeyPress}
268-
aria-haspopup='listbox'
335+
aria-haspopup={true}
269336
aria-expanded={isOpen}
270337
aria-controls='annotations-dropdown-menu'
271338
id='dropdown-button'

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

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import React from 'react';
2-
import { act, fireEvent, render, screen, within } from '@testing-library/react';
2+
import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react';
33
import AnnotationSetSelect from './AnnotationSetSelect';
44
import * as annotationParser from '@Services/annotations-parser';
55

@@ -658,6 +658,56 @@ describe('AnnotationSetSelect component', () => {
658658
// Collapses the dropdown
659659
expect(dropdownMenu.children[0]).not.toHaveClass('open');
660660
});
661+
662+
test('allows a printable character to focus an option starts with it', async () => {
663+
// Dropdown menu is collapsed initially
664+
expect(dropdownMenu).toHaveClass('ramp--annotations__multi-select-header');
665+
expect(dropdownMenu.children[0]).not.toHaveClass('open');
666+
expect(dropdownMenu).toHaveFocus();
667+
668+
fireEvent.keyDown(dropdownMenu, { key: 't', keyCode: 84 });
669+
670+
await waitFor(() => {
671+
expect(dropdownMenu).not.toHaveFocus();
672+
expect(dropdownMenu.children[0]).toHaveClass('open');
673+
expect(multiSelectHeader.childNodes[1].children).toHaveLength(4);
674+
const focusedOption = multiSelectHeader.childNodes[1].children[2];
675+
expect(focusedOption).toHaveFocus();
676+
expect(focusedOption.textContent.toLowerCase().startsWith('t')).toBeTruthy();
677+
});
678+
});
679+
680+
describe('and expanded', () => {
681+
beforeEach(() => {
682+
// Expand the dropdown list
683+
fireEvent.keyDown(dropdownMenu, { key: 'Enter', keyCode: 13 });
684+
});
685+
686+
test('allows Home keypress to focus the first option', () => {
687+
// Dropdown menu is expanded initially
688+
expect(dropdownMenu.children[0]).toHaveClass('open');
689+
expect(dropdownMenu).toHaveFocus();
690+
691+
fireEvent.keyDown(dropdownMenu, { key: 'Home', keyCode: 36 });
692+
693+
// Focus is moved from the dropdown menu to its first option
694+
expect(dropdownMenu).not.toHaveFocus();
695+
expect(multiSelectHeader.childNodes[1].children[0]).toHaveFocus();
696+
});
697+
698+
test('allows End keypress to focus the last option', () => {
699+
// Dropdown menu is expanded initially
700+
expect(dropdownMenu.children[0]).toHaveClass('open');
701+
expect(dropdownMenu).toHaveFocus();
702+
expect(multiSelectHeader.childNodes[1].children).toHaveLength(4);
703+
704+
fireEvent.keyDown(dropdownMenu, { key: 'End', keyCode: 35 });
705+
706+
// Focus is moved from the dropdown menu to its last option
707+
expect(dropdownMenu).not.toHaveFocus();
708+
expect(multiSelectHeader.childNodes[1].children[3]).toHaveFocus();
709+
});
710+
});
661711
});
662712

663713
describe('when focused on an annotation set', () => {
@@ -743,6 +793,48 @@ describe('AnnotationSetSelect component', () => {
743793
expect(multiSelectHeader.childNodes[1]).toHaveClass('ramp--annotations__scroll');
744794
expect(dropdownMenu.children[0]).not.toHaveClass('open');
745795
});
796+
797+
test('allows a printable character to focus an option starts with it', async () => {
798+
// The first option in the dropdown has focus initially
799+
expect(allOptions[0]).toHaveFocus();
800+
801+
fireEvent.keyDown(dropdownMenu, { key: 't', keyCode: 84 });
802+
803+
await waitFor(() => {
804+
expect(allOptions[0]).not.toHaveFocus();
805+
const focusedOption = allOptions[2];
806+
expect(focusedOption).toHaveFocus();
807+
expect(focusedOption.textContent.toLowerCase().startsWith('t')).toBeTruthy();
808+
});
809+
});
810+
811+
test('allows PageUp keypress to focus the first option', () => {
812+
// The first option in the dropdown has focus initially
813+
expect(allOptions[0]).toHaveFocus();
814+
815+
// Move focus to next option in the list
816+
fireEvent.keyDown(allOptions[0], { key: 'ArrowDown', keyCode: 40 });
817+
expect(allOptions[0]).not.toHaveFocus();
818+
expect(allOptions[1]).toHaveFocus();
819+
820+
fireEvent.keyDown(dropdownMenu, { key: 'PageUp', keyCode: 33 });
821+
822+
// Focus is moved from second option to first option
823+
expect(allOptions[0]).toHaveFocus();
824+
expect(allOptions[1]).not.toHaveFocus();
825+
});
826+
827+
test('allows PageDown keypress to focus the last option', () => {
828+
// The first option in the dropdown has focus initially
829+
expect(allOptions[0]).toHaveFocus();
830+
831+
fireEvent.keyDown(dropdownMenu, { key: 'PageDown', keyCode: 34 });
832+
833+
// Focus is moved from first option to last option
834+
expect(allOptions[0]).not.toHaveFocus();
835+
expect(allOptions[3]).toHaveFocus();
836+
837+
});
746838
});
747839
});
748840
});

‎src/services/utility-helpers.js

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,11 @@ const truncateNode = (node, maxLength) => {
882882
}
883883
}
884884

885+
// Set tab-index of each Anchor node to -1 to remove them from tab order of the page
886+
if (node.nodeType === Node.ELEMENT_NODE && node.nodeName.toLowerCase() === 'a') {
887+
node.tabIndex = -1;
888+
}
889+
885890
let currentRemaining = maxLength;
886891
const childNodes = Array.from(node.childNodes);
887892

‎src/services/utility-helpers.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -916,7 +916,7 @@ describe('util helper', () => {
916916
// Character count for text "Bold and superscript text" is 25
917917
const html = '<p><strong>Bold</strong> and <sup><a href="http://example.com">superscript</a></sup> text</p>';
918918
const { isTruncated, truncated } = util.truncateText(html, 15);
919-
expect(truncated).toBe('<p><strong>Bold</strong> and <sup><a href="http://example.com">supers...</a></sup></p>');
919+
expect(truncated).toBe('<p><strong>Bold</strong> and <sup><a href="http://example.com" tabindex="-1">supers...</a></sup></p>');
920920
expect(isTruncated).toBe(true);
921921
});
922922
});

0 commit comments

Comments
 (0)