Skip to content

Commit

Permalink
fix(popup): do not bubble keyboard events to parent
Browse files Browse the repository at this point in the history
  • Loading branch information
Niklas Kiefer authored and nikku committed Sep 20, 2023
1 parent aab9cb3 commit a8dd384
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 9 deletions.
15 changes: 6 additions & 9 deletions src/components/Popup.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,11 @@ export function Popup(props) {
const focusTrapRef = useRef(null);
const popupRef = useRef(null);

const handleKeyPress = event => {
const handleKeydown = event => {

// do not allow keyboard events to bubble
event.stopPropagation();

if (closeOnEscape && event.key === 'Escape') {
onClose();
}
Expand Down Expand Up @@ -85,14 +89,6 @@ export function Popup(props) {
style.height = height + 'px';
}

useEffect(() => {
if (popupRef.current) {
popupRef.current.addEventListener('keydown', handleKeyPress);
}

return () => { popupRef.current.removeEventListener('keydown', handleKeyPress); };
}, [ popupRef ]);

useEffect(() => {
if (popupRef.current) {
popupRef.current.addEventListener('focusin', handleFocus);
Expand Down Expand Up @@ -124,6 +120,7 @@ export function Popup(props) {
aria-label={ title }
tabIndex={ -1 }
ref={ popupRef }
onKeyDown={ handleKeydown }
role="dialog"
class={ classNames('bio-properties-panel-popup', className) }
style={ style }>{ props.children }</div>
Expand Down
26 changes: 26 additions & 0 deletions test/spec/components/FeelPopup.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,32 @@ describe('<FeelPopup>', function() {
});


it('should not bubble keyboard events to parent', async function() {

// given
const keyDownSpy = sinon.spy();

container.addEventListener('keydown', keyDownSpy);

const result = createFeelPopup({ type: 'feel', popupContainer: container }, container);

const childComponent = domQuery('.child-component', container);
const btn = domQuery('button', childComponent);

await act(() => {
btn.click();
});

const editor = getFeelEditor(result.container);

// when
fireEvent.keyDown(domQuery('.cm-content', editor), { key: 'A' });

// then
expect(keyDownSpy).to.not.have.been.called;
});


describe('<feel>', function() {

it('should open <feel> editor', async function() {
Expand Down
22 changes: 22 additions & 0 deletions test/spec/components/Popup.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,28 @@ describe('<Popup>', function() {
});


it('should not bubble keyboard events to parent', async function() {

// given
const keyDownSpy = sinon.spy();

container.addEventListener('keydown', keyDownSpy);

await act(() => {
render(<Popup container={ container }><input name="foo"></input></Popup>, { container });
});

const popup = domQuery('.bio-properties-panel-popup', document.body);
const input = domQuery('input', popup);

// when
fireEvent.keyDown(input, { key: 'A' });

// then
expect(keyDownSpy).to.not.have.been.called;
});


describe('Popup.Title', function() {

it('should render children', function() {
Expand Down

0 comments on commit a8dd384

Please sign in to comment.