fix: prevent flickering from engine update during selection (#1126)

* refactor: listbox selection * fix: selection updates from listbox-toolbar or search * fix: field has changed check, exising layout was modified * test: update unit tests
qlik-oss · Mar 14, 2023 · e2a43c0 · e2a43c0
1 parent 873615e
commit e2a43c0
Show file tree

Hide file tree

Showing 13 changed files with 474 additions and 439 deletions.
diff --git a/apis/nucleus/src/components/listbox/ListBox.jsx b/apis/nucleus/src/components/listbox/ListBox.jsx
@@ -22,6 +22,7 @@ export default function ListBox({
   constraints,
   layout,
   selections,
+  selectionState,
   direction,
   checkboxes: checkboxOption,
   height,
@@ -65,16 +66,23 @@ export default function ListBox({
     postProcessPages,
     listData,
   });
-  const [pages, setPages] = useState([]);
   const { setStoreValue } = useDataStore(model);
   const loadMoreItems = useCallback(itemsLoader.loadMoreItems, [layout]);
 
   const [overflowDisclaimer, setOverflowDisclaimer] = useState({ show: false, dismissed: false });
   const showOverflowDisclaimer = (show) => setOverflowDisclaimer((state) => ({ ...state, show }));
 
-  useEffect(() => {
-    setPages(itemsLoader?.pages || []);
-  }, [itemsLoader?.pages]);
+  const [pages, setPages] = useState([]);
+
+  if (itemsLoader?.pages) {
+    selectionState.update({
+      setPages,
+      pages: itemsLoader?.pages,
+      isSingleSelect,
+      selectDisabled,
+      layout,
+    });
+  }
 
   const isItemLoaded = useCallback(
     (index) => {
@@ -89,18 +97,11 @@ export default function ListBox({
     [layout, pages]
   );
 
-  const {
-    instantPages = [],
-    interactionEvents,
-    select,
-  } = useSelectionsInteractions({
-    layout,
+  const { interactionEvents, select } = useSelectionsInteractions({
+    selectionState,
     selections,
-    pages,
     checkboxes,
-    selectDisabled,
     doc: document,
-    isSingleSelect,
   });
 
   const { layoutOptions = {} } = layout || {};
@@ -143,13 +144,6 @@ export default function ListBox({
     update.call(null, fetchData);
   }
 
-  useEffect(() => {
-    if (!instantPages || isLoadingData) {
-      return;
-    }
-    setPages(instantPages);
-  }, [instantPages]);
-
   useEffect(() => {
     if (scrollState && !initScrollPosIsSet && loaderRef.current) {
       loaderRef.current._listRef.scrollToItem(scrollState.initScrollPos);
@@ -198,7 +192,7 @@ export default function ListBox({
     });
     local.current.dataOffset = offset;
     if (triggerRerender) {
-      setPages((currentPages) => [...currentPages]);
+      selectionState.triggerStateChanged();
     }
     scrollToIndex(scrollIndex);
   };

diff --git a/apis/nucleus/src/components/listbox/ListBoxInline.jsx b/apis/nucleus/src/components/listbox/ListBoxInline.jsx
@@ -15,6 +15,7 @@ import ListBoxSearch from './components/ListBoxSearch';
 import { getListboxInlineKeyboardNavigation } from './interactions/listbox-keyboard-navigation';
 import addListboxTheme from './assets/addListboxTheme';
 import useAppSelections from '../../hooks/useAppSelections';
+import createSelectionState from './hooks/selections/selectionState';
 import { CELL_PADDING_LEFT, ICON_PADDING } from './constants';
 
 const PREFIX = 'ListBoxInline';
@@ -102,6 +103,7 @@ function ListBoxInline({ options, layout }) {
   const updateKeyScroll = (newState) => setKeyScroll((current) => ({ ...current, ...newState }));
   const [currentScrollIndex, setCurrentScrollIndex] = useState({ start: 0, stop: 0 });
   const [appSelections] = useAppSelections(app);
+  const [selectionState] = useState(() => createSelectionState());
 
   const { handleKeyDown, handleOnMouseEnter, handleOnMouseLeave } = getListboxInlineKeyboardNavigation({
     setKeyboardActive,
@@ -176,6 +178,7 @@ function ListBoxInline({ options, layout }) {
         layout,
         model,
         translator,
+        selectionState,
       })
     : [];
 
@@ -294,6 +297,7 @@ function ListBoxInline({ options, layout }) {
           <div className={classes.screenReaderOnly}>{translator.get('Listbox.Search.ScreenReaderInstructions')}</div>
           <ListBoxSearch
             selections={selections}
+            selectionState={selectionState}
             model={model}
             dense={dense}
             keyboard={keyboard}
@@ -314,6 +318,7 @@ function ListBoxInline({ options, layout }) {
                 constraints={constraints}
                 layout={layout}
                 selections={selections}
+                selectionState={selectionState}
                 direction={direction}
                 frequencyMode={frequencyMode}
                 rangeSelect={rangeSelect}

diff --git a/apis/nucleus/src/components/listbox/ListBoxPopover.jsx b/apis/nucleus/src/components/listbox/ListBoxPopover.jsx
@@ -18,6 +18,7 @@ import InstanceContext from '../../contexts/InstanceContext';
 
 import ListBoxSearch from './components/ListBoxSearch';
 import useObjectSelections from '../../hooks/useObjectSelections';
+import createSelectionState from './hooks/selections/selectionState';
 import getHasSelections from './assets/has-selections';
 
 export default function ListBoxPopover({
@@ -89,6 +90,7 @@ export default function ListBoxPopover({
   const containerRef = useRef();
   const [selections] = useObjectSelections(app, model, containerRef);
   const [layout] = useLayout(model);
+  const [selectionState] = useState(() => createSelectionState());
 
   useEffect(() => {
     if (selections && open) {
@@ -114,6 +116,7 @@ export default function ListBoxPopover({
     layout,
     model,
     translator,
+    selectionState,
   });
 
   const hasSelections = getHasSelections(layout);
@@ -173,13 +176,15 @@ export default function ListBoxPopover({
             model={model}
             listCount={listCount}
             selections={selections}
+            selectionState={selectionState}
             keyboard={{ enabled: false }}
             autoFocus={autoFocus ?? true}
           />
           <ListBox
             model={model}
             layout={layout}
             selections={selections}
+            selectionState={selectionState}
             direction="ltr"
             onSetListCount={(c) => setListCount(c)}
           />

diff --git a/apis/nucleus/src/components/listbox/__tests__/list-box.test.jsx b/apis/nucleus/src/components/listbox/__tests__/list-box.test.jsx
@@ -29,6 +29,7 @@ describe('<Listbox />', () => {
   let args;
   let layout;
   let selections;
+  let selectionState;
   let renderer;
   let render;
   let pages;
@@ -55,7 +56,6 @@ describe('<Listbox />', () => {
     };
 
     useSelectionsInteractions = jest.fn().mockReturnValue({
-      instantPages: [],
       interactionEvents: {
         onMouseDown: () => {},
         onMouseUp: () => {},
@@ -104,6 +104,7 @@ describe('<Listbox />', () => {
     global.window = { setTimeout: setTimeoutStub };
     selectDisabled = () => false;
     selections = { key: 'selections' };
+    selectionState = { update: jest.fn() };
 
     args = {
       model: {
@@ -113,6 +114,7 @@ describe('<Listbox />', () => {
       frequencyMode: 'N',
       histogram: false,
       selections,
+      selectionState,
       postProcessPages: undefined,
       calculatePagesHeight: false,
       keyboard: undefined,
@@ -154,6 +156,7 @@ describe('<Listbox />', () => {
                 keyboard={mergedArgs.keyboard}
                 showGray={mergedArgs.showGray}
                 selections={mergedArgs.selections}
+                selectionState={mergedArgs.selectionState}
                 direction={mergedArgs.direction}
                 height={mergedArgs.height}
                 width={mergedArgs.width}
@@ -183,23 +186,18 @@ describe('<Listbox />', () => {
       expect(rows).toHaveLength(1);
       expect(columns).toHaveLength(0);
       expect(useSelectionsInteractions.mock.lastCall[0]).toMatchObject({
-        layout,
         selections,
-        pages: [],
         doc: expect.any(Object),
       });
 
-      expect(useSelectionsInteractions.mock.calls[1][0].selectDisabled instanceof Function).toBe(true);
       expect(FixedSizeList.mock.calls.length).toBeGreaterThan(0);
     });
 
     test('should call useSelectionsInteractions', async () => {
       await render();
 
       expect(useSelectionsInteractions.mock.lastCall[0]).toMatchObject({
-        layout,
         selections,
-        pages: [],
         doc: expect.any(Object),
       });
     });
@@ -229,9 +227,7 @@ describe('<Listbox />', () => {
       await render();
 
       expect(useSelectionsInteractions.mock.lastCall[0]).toMatchObject({
-        layout,
         selections,
-        pages: [],
         doc: expect.any(Object),
       });
 

diff --git a/apis/nucleus/src/components/listbox/components/ListBoxSearch.jsx b/apis/nucleus/src/components/listbox/components/ListBoxSearch.jsx
@@ -20,6 +20,7 @@ const StyledOutlinedInput = styled(OutlinedInput)(({ theme }) => ({
 
 export default function ListBoxSearch({
   selections,
+  selectionState,
   model,
   keyboard,
   dense = false,
@@ -101,6 +102,7 @@ export default function ListBoxSearch({
       response = model.acceptListObjectSearch(TREE_PATH, true);
       // eslint-disable-next-line no-param-reassign
       selections.selectionsMade = true;
+      selectionState.clearItemStates(false);
       setValue('');
     }
     return response;

diff --git a/apis/nucleus/src/components/listbox/components/__tests__/list-box-search.test.jsx b/apis/nucleus/src/components/listbox/components/__tests__/list-box-search.test.jsx
@@ -118,9 +118,12 @@ describe('<ListBoxSearch />', () => {
   });
 
   test('should reset `OutlinedInput` and `acceptListObjectSearch` on `Enter`', async () => {
+    const selectionState = {
+      clearItemStates: jest.fn(),
+    };
     const testRenderer = create(
       <InstanceContext.Provider value={{ translator: { get: () => 'Search' } }}>
-        <ListBoxSearch selections={selections} model={model} keyboard={keyboard} />
+        <ListBoxSearch selections={selections} selectionState={selectionState} model={model} keyboard={keyboard} />
       </InstanceContext.Provider>
     );
     const testInstance = testRenderer.root;
@@ -136,6 +139,8 @@ describe('<ListBoxSearch />', () => {
     });
     expect(model.acceptListObjectSearch).toHaveBeenCalledWith('/qListObjectDef', true);
     expect(type.props.value).toBe('');
+    expect(selectionState.clearItemStates).toBeCalledTimes(1);
+    expect(selectionState.clearItemStates).toBeCalledWith(false);
   });
 
   test('should not accept search result if no hits', async () => {