gristlabs · georgegevoian · Oct 3, 2025 · Aug 6, 2025 · Aug 6, 2025 · Aug 6, 2025
diff --git a/app/client/components/RegionFocusSwitcher.ts b/app/client/components/RegionFocusSwitcher.ts
@@ -5,6 +5,7 @@ import isEqual from 'lodash/isEqual';
 import {makeT} from 'app/client/lib/localization';
 import {FocusLayer} from 'app/client/lib/FocusLayer';
 import {trapTabKey} from 'app/client/lib/trapTabKey';
+import {isFocusable} from 'app/client/lib/isFocusable';
 import * as commands from 'app/client/components/commands';
 import {App} from 'app/client/ui/App';
 import {GristDoc} from 'app/client/components/GristDoc';
@@ -13,7 +14,7 @@ import {components} from 'app/common/ThemePrefs';
 
 const t = makeT('RegionFocusSwitcher');
 
-type Panel = 'left' | 'top' | 'right' | 'main';
+export type Panel = 'left' | 'top' | 'right' | 'main';
 interface PanelRegion {
   type: 'panel',
   id: Panel // this matches a dom element id
@@ -99,38 +100,6 @@ export class RegionFocusSwitcher extends Disposable {
     }
   }
 
-  public focusRegion(
-    region: Region | undefined,
-    options: {initiator?: StateUpdateInitiator} = {}
-  ) {
-    if (region?.type === 'panel' && !getPanelElement(region.id)) {
-      return;
-    }
-
-    const gristDoc = this._getGristDoc();
-    if (gristDoc && region?.type === 'panel' && region?.id === 'main') {
-      console.error('main panel is not supported when a view layout is rendered');
-      return;
-    }
-    if (!gristDoc && region?.type === 'section') {
-      console.error('view section id is not supported when no view layout is rendered');
-      return;
-    }
-
-    this._state.set({region, initiator: options.initiator});
-  }
-
-  public focusActiveSection() {
-    const gristDoc = this._getGristDoc();
-    if (gristDoc) {
-      this.focusRegion({type: 'section', id: gristDoc.viewModel.activeSectionId()});
-    }
-  }
-
-  public reset() {
-    this.focusRegion(undefined);
-  }
-
   public panelAttrs(id: Panel, ariaLabel: string) {
     return [
       dom.attr('role', id === 'main'
@@ -170,10 +139,89 @@ export class RegionFocusSwitcher extends Disposable {
     ];
   }
 
+  /**
+   * Get a normalized region id for the given region (or the current region if none given).
+   *
+   * Note: an active section, which is a grist doc view section, is exposed as the 'main' region.
+   */
+  public getRegionId(region?: Region) {
+    const state = region ?? this._state.get().region;
+    if (state?.type === 'panel') {
+      return state.id;
+    }
+    return 'main';
+  }
+
+  /**
+   * Focus a given region by id.
+   *
+   * If we want to focus the 'main' region on a grist doc, we actually focus the active view section.
+   */
+  public focusRegion(id: Panel) {
+    const gristDoc = this._getGristDoc();
+    if (gristDoc && id === 'main') {
+      this.focusActiveSection();
+      return;
+    }
+    this._focusRegion({type: 'panel', id});
+  }
+
+  /**
+   * Focus the active section of the current grist doc.
+   *
+   * Difference with `focusRegion('main')` is that focus won't change if don't detect any active section.
+   */
+  public focusActiveSection() {
+    const gristDoc = this._getGristDoc();
+    if (gristDoc) {
+      this._focusRegion({type: 'section', id: gristDoc.viewModel.activeSectionId()});
+    }
+  }
+
+  /**
+   * Add a listener to the current region change.
+   * Exposes only the normalized region ids (current and previous)
+   */
+  public addListener(listener: (regionId: Panel, prevRegionId: Panel) => void) {
+    return this._state.addListener((state, prev) => {
+      const currentRegionId = this.getRegionId(state.region);
+      const prevRegionId = this.getRegionId(prev.region);
+      if (currentRegionId === prevRegionId) {
+        return;
+      }
+      listener(currentRegionId, prevRegionId);
+    });
+  }
+
+  public reset() {
+    this._focusRegion(undefined);
+  }
+
+  private _focusRegion(
+    region: Region | undefined,
+    options: {initiator?: StateUpdateInitiator} = {}
+  ) {
+    if (region?.type === 'panel' && !getPanelElement(region.id)) {
+      return;
+    }
+
+    const gristDoc = this._getGristDoc();
+    if (gristDoc && region?.type === 'panel' && region?.id === 'main') {
+      console.error('main panel is not supported when a view layout is rendered');
+      return;
+    }
+    if (!gristDoc && region?.type === 'section') {
+      console.error('view section id is not supported when no view layout is rendered');
+      return;
+    }
+
+    this._state.set({region, initiator: options.initiator});
+  }
+
   private _cycle(direction: 'next' | 'prev') {
     const gristDoc = this._getGristDoc();
     const cycleRegions = getCycleRegions(gristDoc);
-    this.focusRegion(getSibling(
+    this._focusRegion(getSibling(
       this._state.get().region,
       cycleRegions,
       direction,
@@ -200,18 +248,27 @@ export class RegionFocusSwitcher extends Disposable {
     const targetsMain = targetRegionId === 'main';
 
     // When not targeting the main panel, we don't always want to focus the given region _on click_.
+    //
     // We only do it if clicking an empty area in the panel, or a focusable element like an input.
-    // Otherwise, we assume clicks are on elements like buttons or links,
-    // and we don't want to lose focus of the main section in that case.
-    // For example I don't want to focus out current table if I just click the "undo" button in the header.
+    // Because we kind of expect these behaviors usually on the web: I click on
+    // an empty space, and I can start using Tab to navigate around the area I clicked ;
+    // I click inside an input, and I can use Tab to navigate to the following ones.
+    //
+    // Otherwise, we assume[*] clicks are on elements like buttons or links,
+    // and we don't want to lose focus of current section in this case.
+    // For example I don't want to focus out current table if just click the "undo" button in the header.
+    //
+    // [*]: for now, we "assume" because lots of interactive elements in Grist are divs with click handlers.
+    // So we can't reliably consider that clicking on a div is clicking on a "empty area".
+    // Ideally (WIP) we'd have a more reliable way to detect "buttons" and this code could be simplified.
     const isFocusableElement = isMouseFocusableElement(event.target) || closestRegion === event.target;
 
     if (targetsMain || !isFocusableElement) {
       // don't specify a section id here: we just want to focus back the view layout,
       // we don't specifically know which section, the view layout will take care of that.
-      this.focusRegion({type: 'section'}, {initiator: {type: 'mouse', event}});
+      this._focusRegion({type: 'section'}, {initiator: {type: 'mouse', event}});
     } else {
-      this.focusRegion({type: 'panel', id: targetRegionId as Panel}, {initiator: {type: 'mouse', event}});
+      this._focusRegion({type: 'panel', id: targetRegionId as Panel}, {initiator: {type: 'mouse', event}});
     }
   }
 
@@ -341,13 +398,13 @@ export class RegionFocusSwitcher extends Disposable {
     const current = this._state.get().region;
     const gristDoc = this._getGristDoc();
     if (current?.type === 'panel' && current.id === 'right') {
-      return this.focusRegion(
+      return this._focusRegion(
         gristDoc ? {type: 'section'} : {type: 'panel', id: 'main'},
         {initiator: {type: 'cycle'}}
       );
     }
     commands.allCommands.rightPanelOpen.run();
-    return this.focusRegion({type: 'panel', id: 'right'}, {initiator: {type: 'cycle'}});
+    return this._focusRegion({type: 'panel', id: 'right'}, {initiator: {type: 'cycle'}});
   }
 
   private _canTabThroughMainRegion(use: UseCBOwner) {
@@ -461,8 +518,8 @@ const focusPanel = (panel: PanelRegion, child: HTMLElement | null, gristDoc: Gri
   }
   enableFocusLock(panelElement);
 
-  // Child element found: focus it
-  if (child && child !== panelElement && child.isConnected) {
+  // Child element found: focus it if we actually can
+  if (child && child !== panelElement && child.isConnected && isFocusable(child)) {
     // Visually highlight the element with similar styles than panel focus,
     // only for this time. This is here just to help the user better see the visual change when he switches panels.
     child.setAttribute(ATTRS.focusedElement, 'true');

diff --git a/app/client/components/ViewLayout.ts b/app/client/components/ViewLayout.ts
@@ -201,17 +201,25 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
       printSection: () => { printViewSection(this.layout, this.viewModel.activeSection()).catch(reportError); },
       sortFilterMenuOpen: (sectionId?: number) => { this._openSortFilterMenu(sectionId); },
       expandSection: () => { this._expandSection(); },
+    };
+    // Register the cancel command only when necessary to prevent collapsing with other common "escape" usages.
+    // See commit message for detailed description of why it's important to deal with that this way, instead of simply
+    // testing whether this.maximized.get() is null in a cancel command registered through the commandGroup object.
+    const whenMaximizedCommandGroup = {
       cancel: () => {
-        if (this.maximized.get()) {
-          this.maximized.set(null);
-        }
+        this.maximized.set(null);
       }
     };
     this.autoDispose(commands.createGroup(
       commandGroup,
       this,
       ko.pureComputed(() => this.viewModel.focusedRegionState() === 'in')
     ));
+    this.autoDispose(commands.createGroup(
+      whenMaximizedCommandGroup,
+      this,
+      ko.pureComputed(() => this.viewModel.focusedRegionState() === 'in' && this.layout.maximizedLeaf() !== null)
+    ));
 
     this.maximized = fromKo(this.layout.maximizedLeaf) as any;
     this.autoDispose(this.maximized.addListener((sectionId, prev) => {

diff --git a/app/client/components/commandList.ts b/app/client/components/commandList.ts
@@ -9,6 +9,7 @@ export type CommandName =
   | 'find'
   | 'findNext'
   | 'findPrev'
+  | 'closeSearchBar'
   | 'historyBack'
   | 'historyForward'
   | 'reloadPlugins'
@@ -193,6 +194,11 @@ export const groups: CommendGroupDef[] = [{
       name: 'findPrev',
       keys: ['Mod+Shift+G'],
       desc: 'Find previous occurrence',
+    }, {
+      name: 'closeSearchBar',
+      keys: ['Mod+Enter'],
+      desc: 'When in the search bar, close it and focus the current match',
+      alwaysOn: true,
     }, {
       // Without this, when focus in on Clipboard, this shortcut would only move the cursor.
       name: 'historyBack',

diff --git a/app/client/lib/isFocusable.ts b/app/client/lib/isFocusable.ts
@@ -0,0 +1,87 @@
+
+/**
+ * Code authored by Kitty Giraudel for a11y-dialog https://github.com/KittyGiraudel/a11y-dialog, thanks to her!
+ *
+ * This was split from the trapTabKey file to expose the `isFocusable` function that is useful on its own.
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2025 Kitty Giraudel
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated
+ * documentation files (the "Software"), to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
+ * and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in all
+ * copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
+ * TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF
+ * CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+/**
+ * Determine if an element is focusable and has user-visible painted dimensions.
+ */
+export const isFocusable = (el: HTMLElement) => {
+  // A shadow host that delegates focus will never directly receive focus,
+  // even with `tabindex=0`. Consider our <fancy-button> custom element, which
+  // delegates focus to its shadow button:
+  //
+  // <fancy-button tabindex="0">
+  //  #shadow-root
+  //  <button><slot></slot></button>
+  // </fancy-button>
+  //
+  // The browser acts as as if there is only one focusable element – the shadow
+  // button. Our library should behave the same way.
+  if (el.shadowRoot?.delegatesFocus) { return false; }
+
+  return el.matches(focusableSelectorsString) && !isHidden(el);
+};
+
+/**
+ * Determine if an element is hidden from the user.
+ */
+const isHidden = (el: HTMLElement) => {
+  // Browsers hide all non-<summary> descendants of closed <details> elements
+  // from user interaction, but those non-<summary> elements may still match our
+  // focusable-selectors and may still have dimensions, so we need a special
+  // case to ignore them.
+  if (
+    el.matches('details:not([open]) *') &&
+    !el.matches('details>summary:first-of-type')
+  )
+    { return true; }
+
+  // If this element has no painted dimensions, it's hidden.
+  return !(el.offsetWidth || el.offsetHeight || el.getClientRects().length);
+};
+
+const notInert = ':not([inert]):not([inert] *)';
+const notNegTabIndex = ':not([tabindex^="-"])';
+const notDisabled = ':not(:disabled)';
+
+const focusableSelectors = [
+  `a[href]${notInert}${notNegTabIndex}`,
+  `area[href]${notInert}${notNegTabIndex}`,
+  `input:not([type="hidden"]):not([type="radio"])${notInert}${notNegTabIndex}${notDisabled}`,
+  `input[type="radio"]${notInert}${notNegTabIndex}${notDisabled}`,
+  `select${notInert}${notNegTabIndex}${notDisabled}`,
+  `textarea${notInert}${notNegTabIndex}${notDisabled}`,
+  `button${notInert}${notNegTabIndex}${notDisabled}`,
+  `details${notInert} > summary:first-of-type${notNegTabIndex}`,
+  // Discard until Firefox supports `:has()`
+  // See: https://github.com/KittyGiraudel/focusable-selectors/issues/12
+  // `details:not(:has(> summary))${notInert}${notNegTabIndex}`,
+  `iframe${notInert}${notNegTabIndex}`,
+  `audio[controls]${notInert}${notNegTabIndex}`,
+  `video[controls]${notInert}${notNegTabIndex}`,
+  `[contenteditable]${notInert}${notNegTabIndex}`,
+  `[tabindex]${notInert}${notNegTabIndex}`,
+];
+
+const focusableSelectorsString = focusableSelectors.join(',');