From 5ff152f0898afc46fad7b3b9e3213f0bcc0919dd Mon Sep 17 00:00:00 2001 From: Nico Rehwaldt Date: Mon, 21 Nov 2022 22:33:34 +0100 Subject: [PATCH] feat(popup-menu): trap keyboard (ESC, navigation) without search Closes #701 --- lib/features/popup-menu/PopupMenuComponent.js | 25 +++++-- .../popup-menu/PopupMenuComponentSpec.js | 65 ++++++++++++++++++- 2 files changed, 83 insertions(+), 7 deletions(-) diff --git a/lib/features/popup-menu/PopupMenuComponent.js b/lib/features/popup-menu/PopupMenuComponent.js index a38501221..975db17ef 100644 --- a/lib/features/popup-menu/PopupMenuComponent.js +++ b/lib/features/popup-menu/PopupMenuComponent.js @@ -9,7 +9,8 @@ import { } from '../../ui'; import { - closest as domClosest + closest as domClosest, + matches as domMatches } from 'min-dom'; import PopupMenuList from './PopupMenuList'; @@ -99,7 +100,6 @@ export default function PopupMenuComponent(props) { const handleKeyDown = event => { if (event.key === 'Escape') { event.preventDefault(); - event.stopPropagation(); return onClose(); } @@ -159,7 +159,9 @@ export default function PopupMenuComponent(props) { }, [ onSelect, onClose, selectedEntry, keyboardSelect ]); const handleKey = useCallback(event => { - setValue(() => event.target.value); + if (domMatches(event.target, 'input')) { + setValue(() => event.target.value); + } }, [ setValue ]); const displayHeader = props.title || Object.keys(headerEntries).length > 0; @@ -167,6 +169,8 @@ export default function PopupMenuComponent(props) { return html` <${PopupMenuWrapper} onClose=${ onClose } + onKeyup=${ handleKey } + onKeydown=${ handleKeyDown } className=${ className } position=${position} width=${ width } @@ -200,8 +204,6 @@ export default function PopupMenuComponent(props) { ` } @@ -229,11 +231,15 @@ export default function PopupMenuComponent(props) { function PopupMenuWrapper(props) { const { onClose, + onKeydown, + onKeyup, className, children, position: positionGetter } = props; + const popupRef = useRef(); + const checkClose = useCallback((event) => { const overlay = domClosest(event.target, '.djs-popup .djs-popup-overlay', true); @@ -259,10 +265,19 @@ function PopupMenuWrapper(props) { overlayEl.style.top = `${position.y}px`; }, [ overlayRef.current, positionGetter ]); + // focus popup initially, on mount + useLayoutEffect(() => { + popupRef.current && popupRef.current.focus(); + }, []); + return html`
', function() { })); + describe('should focus', function() { + + it('with search', inject(function() { + + // when + createPopupMenu({ + container, + search: true, + entries: [ + { id: '1', label: 'Entry 1' }, + { id: '2', label: 'Entry 2' }, + { id: '3', label: 'Entry 3' }, + { id: '4', label: 'Entry 4' }, + { id: '5', label: 'Entry 5' }, + { id: '6', label: 'Last' } + ] + }); + + const searchInputEl = domQuery( + '.djs-popup-search input', container + ); + + // then + expect(document.activeElement).to.equal(searchInputEl); + })); + + + it('without search', inject(function() { + + // when + createPopupMenu({ + container + }); + + const popupEl = domQuery( + '.djs-popup', container + ); + + // then + expect(document.activeElement).to.equal(popupEl); + })); + + }); + + it('should apply custom width', inject(function() { // when @@ -141,7 +186,7 @@ describe('features/popup-menu - ', function() { describe('body', function() { - it('should focus first entry', inject(function() { + it('should select first entry', inject(function() { const entries = [ { id: '1', label: 'Entry 1' }, { id: '2', label: 'Entry 2' } @@ -186,7 +231,7 @@ describe('features/popup-menu - ', function() { ]; - it('should filter entries + focus first', inject(async function() { + it('should filter entries + select first', inject(async function() { // given createPopupMenu({ container, entries, search: true }); @@ -319,6 +364,22 @@ describe('features/popup-menu - ', function() { })); + it('on popup', inject(function() { + + // given + const onClose = sinon.spy(); + createPopupMenu({ container, entries, onClose, search: true }); + + const popupEl = domQuery('.djs-popup', container); + + // when + popupEl.dispatchEvent(keyDown('Escape')); + + // then + expect(onClose).to.be.calledOnce; + })); + + it('global', inject(async function() { // given