(kb) introduce panels keyboard navigation with new commands

manuhabitela · manuhabitela · commit 9ce2d5ca2fbb · 2025-02-11T13:18:35.000+01:00
1) Add nextPanel and prevPanel commands that are bound on the same key
as nextSection and prevSection commands.

On grist document pages:
new behavior cycles through page panels in addition to page widgets when
pressing the keybinding. Pressing Ctrl+o will first trigger the
nextSection command as usual. *But*, when we are on the last widget and
press Ctrl+o, instead of cycling back to the first one, it will go focus
the left sidebar. Then the top bar. Then go back to the first widget.
Etc.

On other pages:
pressing Ctrl+o now cycles through the page panels to allow quick
keyboard navigation between the app parts. It cycles through left panel,
top panel and main content area.

2) Add creatorPanel command to keyboard focus from/to the creator panel.
This is a separate command from the panels-cycling, as the creator panel
is tied to the currently focused viewLayout section. So we must be able
to focus a section, and then directly go to the creator panel in order
to configure it.

3) ViewModel now keeps track of whether the view "panel" is the
currently focused one or not, and this is now a condition to consider a
viewSection "hasFocus" or not. This allows to easily disable viewSection
commands when focusing app panels.

This is not done yet:
- needs more actual usage testing to finetune the behavior and make sure
it is what we want. The ViewModel change might be too much
- I need to rework the creatorPanel stuff as it doesn't take into
account the ViewModel focus
- I need to check every page using the `pagePanels` function, as a new
div wraps the content and may make the layout buggy
- needs tests
diff --git a/app/client/components/Clipboard.js b/app/client/components/Clipboard.js
@@ -305,12 +305,17 @@ async function getTextFromClipboardItem(clipboardItem, type) {
 /**
  * Helper to determine if the currently active element deserves to keep its own focus, and capture
  * copy-paste events. Besides inputs and textareas, any element can be marked to be a valid
- * copy-paste target by adding 'clipboard_focus' class to it.
+ * copy-paste target by adding 'clipboard_focus' class to it. You can explicitly disallow focus
+ * on an element by adding the 'clipboard_ignore' class to it.
  */
 function allowFocus(elem) {
-  return elem && (FOCUS_TARGET_TAGS.hasOwnProperty(elem.tagName) ||
-    elem.hasAttribute("tabindex") ||
-    elem.classList.contains('clipboard_focus'));
+  return elem
+    && !elem.classList.contains('clipboard_ignore')
+    && (
+      FOCUS_TARGET_TAGS.hasOwnProperty(elem.tagName) ||
+      elem.hasAttribute("tabindex") ||
+      elem.classList.contains('clipboard_focus')
+    );
 }
 
 Clipboard.allowFocus = allowFocus;
diff --git a/app/client/components/ViewLayout.ts b/app/client/components/ViewLayout.ts
@@ -197,6 +197,17 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
       deleteSection: () => { this.removeViewSection(this.viewModel.activeSectionId()).catch(reportError); },
       nextSection: () => { this._otherSection(+1); },
       prevSection: () => { this._otherSection(-1); },
+      // todo: transform firstSection/lastSection to setSection so that creatorPanel can use it to toggle on/off and profit from isFocusedPanel ?
+      firstSection: () => {
+        this.viewModel.isFocusedPanel(true);
+        const sectionIds = this.layout.getAllLeafIds();
+        this.viewModel.activeSectionId(sectionIds[0]);
+      },
+      lastSection: () => {
+        this.viewModel.isFocusedPanel(true);
+        const sectionIds = this.layout.getAllLeafIds();
+        this.viewModel.activeSectionId(sectionIds[sectionIds.length - 1]);
+      },
       printSection: () => { printViewSection(this.layout, this.viewModel.activeSection()).catch(reportError); },
       sortFilterMenuOpen: (sectionId?: number) => { this._openSortFilterMenu(sectionId); },
       expandSection: () => { this._expandSection(); },
@@ -401,13 +412,23 @@ export class ViewLayout extends DisposableWithEvents implements IDomComponent {
 
   // Select another section in cyclic ordering of sections. Order is counter-clockwise if given a
   // positive `delta`, clockwise otherwise.
+  // If requesting an out of bounds section, we redirect to focusing another page panel.
   private _otherSection(delta: number) {
     const sectionIds = this.layout.getAllLeafIds();
     const sectionId = this.viewModel.activeSectionId.peek();
     const currentIndex = sectionIds.indexOf(sectionId);
     const index = mod(currentIndex + delta, sectionIds.length);
-    // update the active section id
-    this.viewModel.activeSectionId(sectionIds[index]);
+
+    if (currentIndex + delta === -1) {
+      this.viewModel.isFocusedPanel(false);
+      commands.allCommands.prevPanel.run();
+    } else if (currentIndex + delta === sectionIds.length) {
+      this.viewModel.isFocusedPanel(false);
+      commands.allCommands.nextPanel.run();
+    } else {
+      this.viewModel.isFocusedPanel(true);
+      this.viewModel.activeSectionId(sectionIds[index]);
+    }
   }
 
   private _maybeFocusInSection()  {
diff --git a/app/client/components/commandList.ts b/app/client/components/commandList.ts
@@ -45,6 +45,11 @@ export type CommandName =
   | 'prevPage'
   | 'nextSection'
   | 'prevSection'
+  | 'firstSection'
+  | 'lastSection'
+  | 'nextPanel'
+  | 'prevPanel'
+  | 'creatorPanel'
   | 'shiftDown'
   | 'shiftUp'
   | 'shiftRight'
@@ -375,11 +380,35 @@ export const groups: CommendGroupDef[] = [{
     }, {
       name: 'nextSection',
       keys: ['Mod+o'],
-      desc: 'Activate next page widget',
+      desc: 'Focus next page widget or panel',
     }, {
       name: 'prevSection',
       keys: ['Mod+Shift+O'],
-      desc: 'Activate previous page widget',
+      desc: 'Focus previous page widget or panel',
+    }, {
+      // This is superseeded by nextSection on pages with view layouts,
+      // but keys mapping is important for pages without view layouts.
+      name: 'nextPanel',
+      keys: ['Mod+o'],
+      desc: '',
+    }, {
+      // This is superseeded by prevSection on pages with view layouts,
+      // but keys mapping is important for pages without view layouts.
+      name: 'prevPanel',
+      keys: ['Mod+Shift+O'],
+      desc: '',
+    }, {
+      name: 'firstSection',
+      keys: [],
+      desc: '',
+    }, {
+      name: 'lastSection',
+      keys: [],
+      desc: '',
+    }, {
+      name: 'creatorPanel',
+      keys: ['Mod+Alt+o'],
+      desc: 'Toggle creator panel keyboard focus',
     }
   ],
 }, {
diff --git a/app/client/models/entities/ViewRec.ts b/app/client/models/entities/ViewRec.ts
@@ -19,6 +19,11 @@ export interface ViewRec extends IRowModel<"_grist_Views"> {
   // This is active collapsed section id. Set when the widget is clicked.
   activeCollapsedSectionId: ko.Observable<number>;
 
+  // Whether the view content area is the currently keyboard-focused "panel".
+  // This changes when we navigate through panels with nextPanel/prevPanel commands.
+  // It is used to toggle the sections keyboard commands accordingly.
+  isFocusedPanel: ko.Observable<boolean>;
+
   // Saved collapsed sections.
   collapsedSections: ko.Computed<number[]>;
 
@@ -41,6 +46,7 @@ export function createViewRec(this: ViewRec, docModel: DocModel): void {
   this.layoutSpecObj = modelUtil.jsonObservable(this.layoutSpec);
 
   this.activeCollapsedSectionId = ko.observable(0);
+  this.isFocusedPanel = ko.observable(true);
 
   this.collapsedSections = this.autoDispose(ko.pureComputed(() => {
     const allSections = new Set(this.viewSections().all().map(x => x.id()));
diff --git a/app/client/models/entities/ViewSectionRec.ts b/app/client/models/entities/ViewSectionRec.ts
@@ -703,7 +703,10 @@ export function createViewSectionRec(this: ViewSectionRec, docModel: DocModel):
 
   this.hasFocus = ko.pureComputed({
     // Read may occur for recently disposed sections, must check condition first.
-    read: () => !this.isDisposed() && this.view().activeSectionId() === this.id(),
+    read: () =>
+      !this.isDisposed()
+      && this.view().activeSectionId() === this.id()
+      && this.view().isFocusedPanel(),
     write: (val) => { this.view().activeSectionId(val ? this.id() : 0); }
   });
 
diff --git a/app/client/ui/AppUI.ts b/app/client/ui/AppUI.ts
@@ -184,5 +184,7 @@ function pagePanelsDoc(owner: IDisposableOwner, appModel: AppModel, appObj: App)
     contentTop: buildDocumentBanners(pageModel),
     contentBottom: dom.create(createBottomBarDoc, pageModel, leftPanelOpen, rightPanelOpen),
     banner: dom.create(ViewAsBanner, pageModel),
+  }, {
+    cycleThroughMain: false,
   });
 }
diff --git a/app/client/ui/PagePanels.ts b/app/client/ui/PagePanels.ts
@@ -3,12 +3,14 @@
  */
 import {makeT} from 'app/client/lib/localization';
 import * as commands from 'app/client/components/commands';
-import {watchElementForBlur} from 'app/client/lib/FocusLayer';
+import {FocusLayer, watchElementForBlur} from 'app/client/lib/FocusLayer';
 import {urlState} from "app/client/models/gristUrlState";
 import {resizeFlexVHandle} from 'app/client/ui/resizeHandle';
 import {hoverTooltip} from 'app/client/ui/tooltips';
 import {transition, TransitionWatcher} from 'app/client/ui/transitions';
-import {cssHideForNarrowScreen, isScreenResizing, mediaNotSmall, mediaSmall, theme} from 'app/client/ui2018/cssVars';
+import {
+  cssHideForNarrowScreen, isScreenResizing, mediaNotSmall, mediaSmall, theme, vars
+} from 'app/client/ui2018/cssVars';
 import {isNarrowScreenObs} from 'app/client/ui2018/cssVars';
 import {icon} from 'app/client/ui2018/icons';
 import {
@@ -50,7 +52,21 @@ export interface PageContents {
   contentBottom?: DomElementArg;
 }
 
-export function pagePanels(page: PageContents) {
+interface PagePanelsOptions {
+  /**
+   * If true, the main pane is included in the focus-cycle of panels through the nextPanel/prevPanel commands.
+   *
+   * Default is true. Useful to disable when the main pane handles its own focus internally, like a grist doc.
+   */
+  cycleThroughMain?: boolean;
+}
+type FocusablePanelId = 'page-panel--left' | 'page-panel--top' | 'page-panel--right' | 'page-panel--main' | null;
+type CyclePanelId = Exclude<FocusablePanelId, 'page-panel--right'>;
+
+export function pagePanels(
+  page: PageContents,
+  options: PagePanelsOptions = { cycleThroughMain: true }
+) {
   const testId = page.testId || noTestId;
   const left = page.leftPanel;
   const right = page.rightPanel;
@@ -94,7 +110,98 @@ export function pagePanels(page: PageContents) {
     (left.panelOpen as SessionObs<boolean>)?.pauseSaving?.(yesNo);
   };
 
+  const cycleThroughMain = options.cycleThroughMain;
+  const cyclePanelIds: CyclePanelId[] = cycleThroughMain
+    ? ['page-panel--left', 'page-panel--top', 'page-panel--main']
+    : ['page-panel--left', 'page-panel--top'];
+  const focusedPanel: {
+    id: Observable<FocusablePanelId>;
+    focusedDomElement: Element | null;
+  } = {
+    id: Observable.create<FocusablePanelId>(null, null),
+    focusedDomElement: null,
+  };
+
+  let focusLayer: FocusLayer | null = null;
+  const prevFocusedElements: Record<Exclude<FocusablePanelId, null>,
+    Element | null
+  > = {
+    'page-panel--left': null,
+    'page-panel--top': null,
+    'page-panel--right': null,
+    'page-panel--main': null,
+  };
+
+  focusedPanel.id.addListener((current, prev) => {
+    // Clean previously set focus layer any time the panel changes.
+    if (focusLayer && !focusLayer.isDisposed()) {
+      focusLayer.dispose();
+    }
+
+    // Save previous panel/main pane previously focused element for later.
+    // If 'prev' is null, it means we're switching from the main pane when cycleThroughMain is false.
+    prevFocusedElements[prev || 'page-panel--main'] = document.activeElement;
+
+    // Create a new focus layer if we're switching to a panel.
+    // Note that when cycleThroughMain is false, 'current' is null when it's the main pane,
+    // so we don't handle focus in that case
+    if (current) {
+      focusLayer = FocusLayer.create(null, {
+        defaultFocusElem: document.getElementById(current) as HTMLDivElement,
+        allowFocus: () => true,
+      });
+    }
+
+    // setTimeout trick is there to prevent a race condition with the FocusLayer change and make sure this is done last.
+    setTimeout(() => {
+      const elementToFocusBack = current ? prevFocusedElements[current] : prevFocusedElements['page-panel--main'];
+      if (elementToFocusBack instanceof HTMLElement) {
+        elementToFocusBack.focus();
+      }
+    }, 0);
+  });
+
+  const goToPanel = (direction: 'next' | 'prev') => {
+    const focusedPanelId = focusedPanel.id.get();
+
+    if (focusedPanelId === 'page-panel--right') {
+      return toggleCreatorPanelFocus();
+    }
+
+    let newIndex = null;
+    if (focusedPanelId === null) {
+      newIndex = direction === 'next' ? 0 : cyclePanelIds.length - 1;
+    } else {
+      const index = cyclePanelIds.indexOf(focusedPanelId);
+      newIndex = index + (direction === 'next' ? 1 : -1);
+    }
+    if (newIndex === (direction === 'next' ? cyclePanelIds.length : -1)) {
+      focusedPanel.id.set(null);
+      commands.allCommands[direction === 'next' ? 'firstSection' : 'lastSection'].run();
+    } else {
+      focusedPanel.id.set(cyclePanelIds[newIndex]);
+    }
+  };
+
+
+  // todo: make viewModel.isFocusedPanel() understand the switch
+  let prev: FocusablePanelId = null;
+  const toggleCreatorPanelFocus = () => {
+    if (!right?.panelOpen.get()) {
+      right?.panelOpen.set(true);
+    }
+    if (focusedPanel.id.get() !== 'page-panel--right') {
+      prev = focusedPanel.id.get();
+      focusedPanel.id.set('page-panel--right');
+    } else {
+      focusedPanel.id.set(prev);
+    }
+  };
+
   const commandsGroup = commands.createGroup({
+    nextPanel: () => goToPanel('next'),
+    prevPanel: () => goToPanel('prev'),
+    creatorPanel: toggleCreatorPanelFocus,
     leftPanelOpen: () => new Promise((resolve) => {
       const watcher = new TransitionWatcher(leftPaneDom);
       watcher.onDispose(() => resolve(undefined));
@@ -136,6 +243,12 @@ export function pagePanels(page: PageContents) {
     cssContentMain(
       leftPaneDom = cssLeftPane(
         testId('left-panel'),
+        dom.attr('id', 'page-panel--left'),
+        dom.attr('tabindex', '-1'),
+        dom.cls('clipboard_ignore'),
+        dom.attr('role', 'region'),
+        dom.attr('aria-label', t('Main navigation and document settings (left panel)')),
+        cssFocusedPanel.cls('', use => use(focusedPanel.id) === 'page-panel--left'),
         cssOverflowContainer(
           contentWrapper = cssLeftPanelContainer(
             cssLeftPaneHeader(
@@ -272,6 +385,12 @@ export function pagePanels(page: PageContents) {
       cssMainPane(
         mainHeaderDom = cssTopHeader(
           testId('top-header'),
+          dom.attr('id', 'page-panel--top'),
+          dom.attr('tabindex', '-1'),
+          dom.cls('clipboard_ignore'),
+          dom.attr('role', 'region'),
+          dom.attr('aria-label', t('Document header')),
+          cssFocusedPanel.cls('', use => use(focusedPanel.id) === 'page-panel--top'),
           (left.hideOpener ? null :
             cssPanelOpener('PanelRight', cssPanelOpener.cls('-open', left.panelOpen),
               testId('left-opener'),
@@ -292,7 +411,16 @@ export function pagePanels(page: PageContents) {
           ),
           dom.style('margin-bottom', use => use(bannerHeight) + 'px'),
         ),
-        page.contentMain,
+
+        cssContentMainPane(
+          dom.attr('id', 'page-panel--main'),
+          dom.attr('tabindex', '-1'),
+          dom.cls('clipboard_ignore'),
+          dom.attr('role', 'region'),
+          dom.attr('aria-label', 'Main content'),
+          page.contentMain,
+        ),
+
         cssMainPane.cls('-left-overlap', leftOverlap),
         testId('main-pane'),
       ),
@@ -306,6 +434,12 @@ export function pagePanels(page: PageContents) {
 
         rightPaneDom = cssRightPane(
           testId('right-panel'),
+          dom.attr('id', 'page-panel--right'),
+          dom.attr('tabindex', '-1'),
+          dom.cls('clipboard_ignore'),
+          dom.attr('role', 'region'),
+          dom.attr('aria-label', t('Creator panel (right panel)')),
+          cssFocusedPanel.cls('', use => use(focusedPanel.id) === 'page-panel--right'),
           cssRightPaneHeader(
             right.header,
             dom.style('margin-bottom', use => use(bannerHeight) + 'px')
@@ -402,6 +536,18 @@ const cssContentMain = styled(cssHBox, `
   flex: 1 1 0px;
   overflow: hidden;
 `);
+
+// div wrapping the contentMain passed to pagePanels
+const cssContentMainPane = styled(cssVBox, `
+  flex-grow: 1;
+`);
+
+const cssFocusedPanel = styled('div', `
+  outline: 3px solid ${theme.widgetActiveBorder};
+  z-index: ${vars.focusedPanelZIndex} !important;
+  outline-offset: -3px;
+`);
+
 export const cssLeftPane = styled(cssVBox, `
   position: relative;
   background-color: ${theme.leftPanelBg};
diff --git a/app/client/ui2018/cssVars.ts b/app/client/ui2018/cssVars.ts
@@ -137,6 +137,7 @@ export const vars = {
   insertColumnLineZIndex: new CustomProp('insert-column-line-z-index', '20'),
   emojiPickerZIndex: new CustomProp('modal-z-index', '20'),
   popupSectionBackdropZIndex: new CustomProp('popup-section-backdrop-z-index', '100'),
+  focusedPanelZIndex: new CustomProp('focused-panel-z-index', '900'),
   menuZIndex: new CustomProp('menu-z-index', '999'),
   modalZIndex: new CustomProp('modal-z-index', '999'),
   onboardingBackdropZIndex: new CustomProp('onboarding-backdrop-z-index', '999'),

Original file line number	Diff line number	Diff line change
`@@ -184,5 +184,7 @@ function pagePanelsDoc(owner: IDisposableOwner, appModel: AppModel, appObj: App)`
`184`	`184`	`contentTop: buildDocumentBanners(pageModel),`
`185`	`185`	`contentBottom: dom.create(createBottomBarDoc, pageModel, leftPanelOpen, rightPanelOpen),`
`186`	`186`	`banner: dom.create(ViewAsBanner, pageModel),`
	`187`	`+ }, {`
	`188`	`+ cycleThroughMain: false,`
`187`	`189`	`});`
`188`	`190`	`}`