From 0af501ef3e7b5840a606f233383a89c24d01328c Mon Sep 17 00:00:00 2001 From: Michael Sereniti <31261408+msereniti@users.noreply.github.com> Date: Tue, 9 Apr 2024 15:12:13 +0200 Subject: [PATCH] [86bxrwyxa][popper] fixed fast keyboard navigation issues (#1271) ## Motivation and Context Fast keyboard navigation over closely placed tooltips may cause some tooltips to stay visible even after triger blur. This PR fixes it. ## How has this been tested? I found no way to cover it with tests. I've been testing it on 4 placed next to each other tooltips with hover interaction. ## Types of changes - [x] Bug fix (non-breaking change which fixes an issue). - [ ] New feature (non-breaking change which adds functionality). - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected). - [ ] Nice improve. ## Checklist: - [x] My code follows the code style of this project. - [x] I have updated the documentation accordingly or it's not required. - [x] Unit tests are not broken. - [x] I have added changelog note to corresponding `CHANGELOG.md` file with planned publish date. - [ ] I have added new unit tests on added of fixed functionality. --- semcore/popper/CHANGELOG.md | 20 ++++++++++++++ semcore/popper/__tests__/index.test.jsx | 1 + .../popper/__tests__/stands/disablePortal.tsx | 1 + semcore/popper/src/Popper.jsx | 27 +++++++++++++++++++ semcore/tooltip/__tests__/index.test.jsx | 1 + .../__tests__/tooltip.browser-test.tsx | 3 +++ 6 files changed, 53 insertions(+) diff --git a/semcore/popper/CHANGELOG.md b/semcore/popper/CHANGELOG.md index 4a8aa2209e..11116fbee7 100644 --- a/semcore/popper/CHANGELOG.md +++ b/semcore/popper/CHANGELOG.md @@ -2,12 +2,32 @@ CHANGELOG.md standards are inspired by [keepachangelog.com](https://keepachangelog.com/en/1.0.0/). +## [5.26.1] - 2024-09-04 + +### Fixed + +- Holding mouse over a place where popper appear was preventing popper from disappear back. +- Fast keyboard navigation over multiple closely placed poppers was causing multiple poppers to stay open. + ## [5.26.0] - 2024-03-27 ### Fixed - Select and similar components with disabled portal were not working properly being wrapped into `label` tag. +## [5.25.2] - 2024-04-03 + +### Fixed + +- Holding mouse over a place where popper appear was preventing popper from disappear back. +- Fast keyboard navigation over multiple closely placed poppers was causing multiple poppers to stay open. + +## [5.25.1] - 2024-04-03 + +### Fixed + +- Select with `disabpledPortal` inside a label was opening second time after selecting the option. + ## [5.25.0] - 2024-03-27 ### Changed diff --git a/semcore/popper/__tests__/index.test.jsx b/semcore/popper/__tests__/index.test.jsx index 3fe08eb660..524e6133eb 100644 --- a/semcore/popper/__tests__/index.test.jsx +++ b/semcore/popper/__tests__/index.test.jsx @@ -31,6 +31,7 @@ describe('Popper', () => { , ); + fireEvent.mouseMove(getByTestId('reference')); fireEvent.mouseEnter(getByTestId('reference')); // because wait call onVisibleChange act(() => { diff --git a/semcore/popper/__tests__/stands/disablePortal.tsx b/semcore/popper/__tests__/stands/disablePortal.tsx index 53a284a25a..18cc32d449 100644 --- a/semcore/popper/__tests__/stands/disablePortal.tsx +++ b/semcore/popper/__tests__/stands/disablePortal.tsx @@ -14,6 +14,7 @@ const Demo = () => { + ); }; diff --git a/semcore/popper/src/Popper.jsx b/semcore/popper/src/Popper.jsx index d9d035a330..978fdba903 100644 --- a/semcore/popper/src/Popper.jsx +++ b/semcore/popper/src/Popper.jsx @@ -50,6 +50,13 @@ const useUpdatePopperEveryFrame = (popperRef) => { return handleAnimationFrame; }; +let lastMouseMove = 0; +if (canUseDOM()) { + document.addEventListener('mousemove', () => { + lastMouseMove = Date.now(); + }); +} + const MODIFIERS_OPTIONS = [ 'offset', 'preventOverflow', @@ -274,6 +281,13 @@ class Popper extends Component { return; } } + /** + * When popper appears right under mouse that doesn't move, it gets undesired onMouseEnter event. + * That may cause hover interaction poppers to display two closely placed poppers. + * That check ensures that onMouseEnter means mouse entered the popper, not popper entered the mouse. + */ + if (action === 'onMouseEnter' && Date.now() - lastMouseMove > 100) return; + const now = Date.now(); const focusAction = ['onFocus', 'onKeyboardFocus', 'onFocusCapture'].includes(action); if ( @@ -284,6 +298,19 @@ class Popper extends Component { ) { return; } + if (!visible) { + /** + * When sibling popovers triggers with hover/focus interactions are navigated fast, + * sometimes focus moves into closing popovers and prevents it from closing. It may cause + * multiple popovers to be opened at the same time. + */ + setTimeout(() => { + const node = this.popperRef.current; + if (node?.getAttribute('tabindex') === '0') { + node.setAttribute('tabindex', '-1'); + } + }, 0); + } const currentTarget = e?.currentTarget; this.handlerChangeVisibleWithTimer(visible, e, () => { clearTimeout(this.timerMultiTrigger); diff --git a/semcore/tooltip/__tests__/index.test.jsx b/semcore/tooltip/__tests__/index.test.jsx index 6ea78732b4..bdf60cc80a 100644 --- a/semcore/tooltip/__tests__/index.test.jsx +++ b/semcore/tooltip/__tests__/index.test.jsx @@ -209,6 +209,7 @@ describe('TooltipBase', () => { , ); + fireEvent.mouseMove(getByTestId('trigger')); fireEvent.mouseEnter(getByTestId('trigger')); act(() => { vi.runAllTimers(); diff --git a/semcore/tooltip/__tests__/tooltip.browser-test.tsx b/semcore/tooltip/__tests__/tooltip.browser-test.tsx index 780e616817..429a3a6e5b 100644 --- a/semcore/tooltip/__tests__/tooltip.browser-test.tsx +++ b/semcore/tooltip/__tests__/tooltip.browser-test.tsx @@ -14,6 +14,9 @@ test.describe('Tooltip', () => { await page.mouse.move( triggerRect.x + triggerRect.width / 2, triggerRect.y + triggerRect.height / 2, + { + steps: 5, + }, ); await new Promise((resolve) => setTimeout(resolve, 1000));