changes after feedback

elastic · Dec 15, 2020 · 6111c75 · 6111c75
1 parent 9082efc
commit 6111c75
Show file tree

Hide file tree

Showing 4 changed files with 431 additions and 467 deletions.
diff --git a/.../plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.test.tsx b/.../plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.test.tsx
@@ -98,7 +98,7 @@ describe('ConfigPanel', () => {
       const firstLayerFocusable = component
         .find(LayerPanel)
         .first()
-        .find('button')
+        .find('section')
         .first()
         .instance();
       act(() => {
@@ -119,17 +119,14 @@ describe('ConfigPanel', () => {
       const secondLayerFocusable = component
         .find(LayerPanel)
         .at(1)
-        .find('button')
+        .find('section')
         .first()
         .instance();
       act(() => {
         component.find('[data-test-subj="lnsLayerRemove"]').at(0).simulate('click');
       });
       const focusedEl = document.activeElement;
       expect(focusedEl).toEqual(secondLayerFocusable);
-      expect(focusedEl?.closest('.lnsLayerPanel')?.getAttribute('data-test-subj')).toEqual(
-        'lns-layerPanel-1'
-      );
     });
 
     it('should focus the first layer when removing the second layer', () => {
@@ -143,22 +140,18 @@ describe('ConfigPanel', () => {
       const firstLayerFocusable = component
         .find(LayerPanel)
         .first()
-        .find('button')
+        .find('section')
         .first()
         .instance();
       act(() => {
         component.find('[data-test-subj="lnsLayerRemove"]').at(2).simulate('click');
       });
       const focusedEl = document.activeElement;
       expect(focusedEl).toEqual(firstLayerFocusable);
-      expect(focusedEl?.closest('.lnsLayerPanel')?.getAttribute('data-test-subj')).toEqual(
-        'lns-layerPanel-0'
-      );
     });
 
     it('should focus the added layer', () => {
       (generateId as jest.Mock).mockReturnValue(`second`);
-      jest.useFakeTimers();
       const dispatch = jest.fn((x) => {
         if (x.subType === 'ADD_LAYER') {
           frame.datasourceLayers.second = mockDatasource.publicAPIMock;
@@ -170,9 +163,7 @@ describe('ConfigPanel', () => {
         component.find('[data-test-subj="lnsLayerAddButton"]').first().simulate('click');
       });
       const focusedEl = document.activeElement;
-      expect(focusedEl?.closest('.lnsLayerPanel')?.getAttribute('data-test-subj')).toEqual(
-        'lns-layerPanel-1'
-      );
+      expect(focusedEl?.children[0].getAttribute('data-test-subj')).toEqual('lns-layerPanel-1');
     });
   });
 });
diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/config_panel.tsx
@@ -24,45 +24,17 @@ export const ConfigPanelWrapper = memo(function ConfigPanelWrapper(props: Config
   ) : null;
 });
 
-const getFirstFocusable = (el: HTMLElement | null) => {
-  if (!el) {
-    return null;
-  }
-  const firstFocusable = el.querySelector(
-    'button, [href], input, select, textarea, [tabindex]:not([tabindex="-1"])'
-  );
-  if (!firstFocusable) {
-    return null;
-  }
-  return (firstFocusable as unknown) as { focus: () => void };
-};
-
 function useFocusUpdate(layerIds: string[]) {
-  const [lastUpdated, setLastUpdated] = useState<{
-    type: 'ADD_LAYER' | 'REMOVE_OR_CLEAR_LAYER';
-    id: string;
-  } | null>(null);
+  const [nextFocusedLayerId, setNextFocusedLayerId] = useState<string | null>(null);
   const [layerRefs, setLayersRefs] = useState<Record<string, HTMLElement | null>>({});
-  useEffect(() => {
-    if (!lastUpdated) {
-      return;
-    }
-    let focusable;
-
-    const { type, id } = lastUpdated;
-
-    if (type === 'ADD_LAYER') {
-      focusable = getFirstFocusable(layerRefs[id]);
-    } else if (type === 'REMOVE_OR_CLEAR_LAYER') {
-      const firstLayer = layerIds.length === 1 ? layerIds[0] : layerIds.filter((l) => l !== id)[0];
-      focusable = firstLayer && getFirstFocusable(layerRefs[firstLayer]);
-    }
 
+  useEffect(() => {
+    const focusable = nextFocusedLayerId && layerRefs[nextFocusedLayerId];
     if (focusable) {
       focusable.focus();
-      setLastUpdated(null);
+      setNextFocusedLayerId(null);
     }
-  }, [layerIds, layerRefs, lastUpdated]);
+  }, [layerIds, layerRefs, nextFocusedLayerId]);
 
   const setLayerRef = useCallback((layerId, el) => {
     if (el) {
@@ -75,22 +47,25 @@ function useFocusUpdate(layerIds: string[]) {
 
   const removeLayerRef = useCallback(
     (layerId) => {
-      if (layerIds.length > 1) {
-        setLayersRefs((refs) => {
-          const newLayerRefs = { ...refs };
-          delete newLayerRefs[layerId];
-          return newLayerRefs;
-        });
+      if (layerIds.length <= 1) {
+        return setNextFocusedLayerId(layerId);
       }
-      setLastUpdated({
-        type: 'REMOVE_OR_CLEAR_LAYER',
-        id: layerId,
+
+      const removedLayerIndex = layerIds.findIndex((l) => l === layerId);
+      const nextFocusedLayerIdId =
+        removedLayerIndex === 0 ? layerIds[1] : layerIds[removedLayerIndex - 1];
+
+      setLayersRefs((refs) => {
+        const newLayerRefs = { ...refs };
+        delete newLayerRefs[layerId];
+        return newLayerRefs;
       });
+      return setNextFocusedLayerId(nextFocusedLayerIdId);
     },
     [layerIds]
   );
 
-  return { setLastUpdated, removeLayerRef, setLayerRef };
+  return { setNextFocusedLayerId, removeLayerRef, setLayerRef };
 }
 
 export function LayerPanels(
@@ -108,7 +83,7 @@ export function LayerPanels(
   } = props;
 
   const layerIds = activeVisualization.getLayerIds(visualizationState);
-  const { setLastUpdated, removeLayerRef, setLayerRef } = useFocusUpdate(layerIds);
+  const { setNextFocusedLayerId, removeLayerRef, setLayerRef } = useFocusUpdate(layerIds);
 
   const setVisualizationState = useMemo(
     () => (newState: unknown) => {
@@ -225,10 +200,7 @@ export function LayerPanels(
                       state,
                     }),
                 });
-                setLastUpdated({
-                  type: 'ADD_LAYER',
-                  id,
-                });
+                setNextFocusedLayerId(id);
               }}
               iconType="plusInCircleFilled"
             />