From 662a1366499ae4768b80978f73a8914ecdde1bfd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20=C3=85str=C3=B6m?= Date: Tue, 28 Feb 2023 14:35:26 +0100 Subject: [PATCH] fix: listbox more menu is broken (#1089) * fix: make sure portal popover closes correctly * fix: update more actions correctly * fix: get rid of states * chore: update test --- .../nucleus/src/components/ActionsToolbar.jsx | 22 ++++++++----------- .../src/components/ActionsToolbarMore.jsx | 12 +++++++--- .../src/components/listbox/ListBoxPortal.jsx | 2 +- .../__tests__/list-box-portal.test.jsx | 7 +++++- apis/nucleus/src/hooks/useObjectSelections.js | 4 ++-- rollup.config.js | 1 + 6 files changed, 28 insertions(+), 20 deletions(-) diff --git a/apis/nucleus/src/components/ActionsToolbar.jsx b/apis/nucleus/src/components/ActionsToolbar.jsx index ed1391015..9dd167d09 100644 --- a/apis/nucleus/src/components/ActionsToolbar.jsx +++ b/apis/nucleus/src/components/ActionsToolbar.jsx @@ -115,8 +115,7 @@ function ActionsToolbar({ const { translator, keyboardNavigation } = useContext(InstanceContext); const [showMoreItems, setShowMoreItems] = useState(false); - const [moreEnabled, setMoreEnabled] = useState(more.enabled); - const [moreActions, setMoreActions] = useState(more.actions); + const moreRef = useRef(); const actionsRef = useRef(); const theme = useTheme(); @@ -131,10 +130,6 @@ function ActionsToolbar({ useEffect(() => () => setShowMoreItems(false), [popover.show]); - useEffect(() => { - setMoreEnabled(more.enabled); - }, [more.enabled]); - useEffect(() => { if (!focusHandler) return; @@ -150,7 +145,14 @@ function ActionsToolbar({ focusHandler.on('focus_toolbar_last', focusLast); }, []); - const newActions = useMemo(() => actions.filter((a) => !a.hidden), [actions]); + let moreEnabled = more.enabled; + let moreActions = more.actions; + const newActions = actions.filter((a) => !a.hidden); + if (newActions.length > maxItems) { + const newMoreActions = newActions.splice(-(newActions.length - maxItems) - 1); + moreEnabled = true; + moreActions = [...newMoreActions, ...more.actions]; + } if (!selections.show && newActions.length === 0) return null; @@ -168,12 +170,6 @@ function ActionsToolbar({ action: () => setShowMoreItems(!showMoreItems), }; - if (newActions.length > maxItems) { - const newMoreActions = newActions.splice(-(newActions.length - maxItems) - 1); - setMoreEnabled(true); - setMoreActions([...newMoreActions, ...more.actions]); - } - const tabCallback = // if keyboardNavigation is true, create a callback to handle tabbing from the first/last button in the toolbar that resets focus on the content keyboardNavigation && focusHandler && focusHandler.refocusContent diff --git a/apis/nucleus/src/components/ActionsToolbarMore.jsx b/apis/nucleus/src/components/ActionsToolbarMore.jsx index 352c2de55..b2373434b 100644 --- a/apis/nucleus/src/components/ActionsToolbarMore.jsx +++ b/apis/nucleus/src/components/ActionsToolbarMore.jsx @@ -10,6 +10,10 @@ import useActionState from '../hooks/useActionState'; const PREFIX = 'More'; +const ActionsToolbarMoreElement = { + className: 'njs-action-toolbar-more', +}; + const classes = { icon: `${PREFIX}-icon`, }; @@ -51,10 +55,12 @@ const More = React.forwardRef( ref={ref} open={show} anchorEl={alignTo.current} - getContentAnchorEl={null} - hideBackdrop - style={{ pointerEvents: 'none' }} transitionDuration={0} + slotProps={{ + root: { + className: ActionsToolbarMoreElement.className, + }, + }} anchorOrigin={{ vertical: 'bottom', horizontal: 'left', diff --git a/apis/nucleus/src/components/listbox/ListBoxPortal.jsx b/apis/nucleus/src/components/listbox/ListBoxPortal.jsx index 282bedbe7..df5547684 100644 --- a/apis/nucleus/src/components/listbox/ListBoxPortal.jsx +++ b/apis/nucleus/src/components/listbox/ListBoxPortal.jsx @@ -73,7 +73,7 @@ function ListBoxWrapper({ app, fieldIdentifier, qId, stateName, element, options const selections = hasExternalSelectionsApi ? options.selectionsApi - : useObjectSelections(app, model, elementRef, options)[0]; + : useObjectSelections(app, model, [elementRef, '.njs-action-toolbar-more'], options)[0]; if (!selections || !model) { return null; diff --git a/apis/nucleus/src/components/listbox/__tests__/list-box-portal.test.jsx b/apis/nucleus/src/components/listbox/__tests__/list-box-portal.test.jsx index 8131cc482..7ee903860 100644 --- a/apis/nucleus/src/components/listbox/__tests__/list-box-portal.test.jsx +++ b/apis/nucleus/src/components/listbox/__tests__/list-box-portal.test.jsx @@ -109,7 +109,12 @@ describe('ListBoxPortal', () => { }; const elem = ListBoxPortal({ app, fieldIdentifier, options }); await render(elem); - expect(useObjectSelectionsMock).toHaveBeenCalledWith(app, options.sessionModel, elementRef, options); + expect(useObjectSelectionsMock).toHaveBeenCalledWith( + app, + options.sessionModel, + [elementRef, '.njs-action-toolbar-more'], + options + ); }); }); diff --git a/apis/nucleus/src/hooks/useObjectSelections.js b/apis/nucleus/src/hooks/useObjectSelections.js index 755352c22..3e66c6d73 100644 --- a/apis/nucleus/src/hooks/useObjectSelections.js +++ b/apis/nucleus/src/hooks/useObjectSelections.js @@ -186,11 +186,11 @@ const getClickOutFuncs = ({ elements, objectSelections, options }) => { return { activateClickOut() { document.addEventListener('mousedown', handler); - options.onSelectionActivated?.(); + options?.onSelectionActivated?.(); }, deactivateClickOut() { document.removeEventListener('mousedown', handler); - options.onSelectionDeactivated?.(); + options?.onSelectionDeactivated?.(); }, }; }; diff --git a/rollup.config.js b/rollup.config.js index 5376194c7..3d57905df 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -129,6 +129,7 @@ const config = ({ format = 'umd', debug = false, file, targetPkg }) => { json(), commonjs(), babel({ + babelHelpers: 'bundled', babelrc: false, include: [ '/**/apis/conversion/**',