(kb) change where we initialize RegionFocusSwitcher

- now the RFS is created in the App, I guess it makes more sense here,
because it's also where the Clipboard is, and now we can more easilly
access it across the app if needed…
- …but I have a whole mess when initializing now (more than before), to
make sure panelAttrs are applied correctly after the grist doc loaded…
i'm sure this can be improved somehow?
- no more dirty window.regionFocusSwitcher

Author: manuhabitela

app/client/components/Clipboard.js

@@ -77,9 +77,6 @@ function Clipboard(app) {
     onDefaultFocus: () => {
       this.copypasteField.value = ' ';
       this.copypasteField.select();
-      if (window.gristRegionFocusSwitcher) {
-        window.gristRegionFocusSwitcher.focusActiveSection();
-      }
       this._app.trigger('clipboard_focus');
     },
     onDefaultBlur: () => {

app/client/components/RegionFocusSwitcher.ts

@@ -1,10 +1,11 @@
-import {Disposable, dom, Observable, styled} from 'grainjs';
+import {Disposable, dom, Listener, Observable, styled} from 'grainjs';
 import {mod} from 'app/common/gutil';
 import {SpecialDocPage} from 'app/common/gristUrls';
 import isEqual from 'lodash/isEqual';
 import {makeT} from 'app/client/lib/localization';
 import * as commands from 'app/client/components/commands';
 import {triggerFocusGrab} from 'app/client/components/Clipboard';
+import {App} from 'app/client/ui/App';
 import {GristDoc} from 'app/client/components/GristDoc';
 import {theme} from 'app/client/ui2018/cssVars';
 import BaseView from 'app/client/components/BaseView';
@@ -31,7 +32,8 @@ type UpdateInitiator = {type: 'cycle'} | {type: 'mouse', event?: MouseEvent};
 export class RegionFocusSwitcher extends Disposable {
   // Currently focused region
   public readonly current: Observable<Region | undefined>;
-  private readonly _gristDocObs?: Observable<GristDoc | null>;
+
+  private _gristDocObs: Observable<GristDoc | null>;
   private _currentUpdateInitiator?: UpdateInitiator;
   // Previously focused elements for each panel (not used for view section ids)
   private _prevFocusedElements: Record<Panel, Element | null> = {
@@ -41,18 +43,30 @@ export class RegionFocusSwitcher extends Disposable {
     main: null,
   };
 
-  constructor(gristDocObs?: Observable<GristDoc | null>) {
+  private _initiated: Observable<boolean>;
+  private _initListener?: Listener;
+
+  constructor(private _app: App) {
     super();
     this.current = Observable.create(this, undefined);
-    if (gristDocObs) {
-      this._gristDocObs = gristDocObs;
-    }
+    this._initiated = Observable.create(this, false);
   }
 
   public init(pageContainer: HTMLElement) {
-    // if we have a grist doc, wait for the current grist doc to be ready before doing anything
+    if (this._initiated.get()) {
+      if (this._initListener && !this._initListener.isDisposed()) {
+        this._initListener.dispose();
+      }
+      return;
+    }
+
+    if (this._app.pageModel?.gristDoc) {
+      this._gristDocObs = this._app.pageModel.gristDoc;
+    }
+
+    // if we have a grist doc, wait for it to be ready before doing anything
     if (this._gristDocObs && this._gristDocObs.get() === null) {
-      this._gristDocObs.addListener((doc, prevDoc) => {
+      this._initListener = this._gristDocObs.addListener((doc, prevDoc) => {
         if (doc && prevDoc === null) {
           doc.regionFocusSwitcher = this;
           this.init(pageContainer);
@@ -70,11 +84,17 @@ export class RegionFocusSwitcher extends Disposable {
 
     this.autoDispose(this.current.addListener(this._onCurrentUpdate.bind(this)));
 
+    const focusActiveSection = () => this.focusActiveSection();
+    this._app.on('clipboard_focus', focusActiveSection);
+    this.onDispose(() => this._app.off('clipboard_focus', focusActiveSection));
+
     if (this._gristDocObs) {
       const onClick = this._onClick.bind(this);
       pageContainer.addEventListener('mouseup', onClick);
       this.onDispose(() => pageContainer.removeEventListener('mouseup', onClick));
     }
+
+    this._initiated.set(true);
   }
 
   public focusRegion(
@@ -114,6 +134,7 @@ export class RegionFocusSwitcher extends Disposable {
       dom.attr('aria-label', ariaLabel),
       dom.attr(ATTRS.regionId, id),
       dom.cls('kb-focus-highlighter-group', use => {
+        use(this._initiated);
         // highlight focused elements everywhere except in the grist doc views
         if (id !== 'main') {
           return true;
@@ -128,6 +149,7 @@ export class RegionFocusSwitcher extends Disposable {
         return isSpecialPage(gristDoc);
       }),
       dom.cls('clipboard_group_focus', use => {
+        use(this._initiated);
         const gristDoc = this._gristDocObs ? use(this._gristDocObs) : null;
         // if we are not on a grist doc, whole page is always focusable
         if (!gristDoc) {
@@ -145,6 +167,7 @@ export class RegionFocusSwitcher extends Disposable {
         return isSpecialPage(gristDoc);
       }),
       dom.cls('clipboard_forbid_focus', use => {
+        use(this._initiated);
         // forbid focus only on the main panel when on an actual document view (and not a special page)
         if (id !== 'main') {
           return false;
@@ -162,6 +185,7 @@ export class RegionFocusSwitcher extends Disposable {
         return true;
       }),
       dom.cls(`${cssFocusedPanel.className}-focused`, use => {
+        use(this._initiated);
         const current = use(this.current);
         return this._currentUpdateInitiator?.type === 'cycle' && current?.type === 'panel' && current.id === id;
       }),

app/client/ui/AccountPage.ts

@@ -1,6 +1,7 @@
 import {detectCurrentLang, makeT} from 'app/client/lib/localization';
 import {checkName} from 'app/client/lib/nameUtils';
 import {AppModel, reportError} from 'app/client/models/AppModel';
+import {App} from 'app/client/ui/App';
 import {urlState} from 'app/client/models/gristUrlState';
 import * as css from 'app/client/ui/AccountPageCss';
 import {ApiKey} from 'app/client/ui/ApiKey';
@@ -39,7 +40,7 @@ export class AccountPage extends Disposable {
   private _allowGoogleLogin = Computed.create(this, (use) => use(this._userObs)?.allowGoogleLogin ?? false)
     .onWrite((val) => this._updateAllowGooglelogin(val));
 
-  constructor(private _appModel: AppModel) {
+  constructor(private _appModel: AppModel, private _appObj: App) {
     super();
     this._setPageTitle();
     this._fetchAll().catch(reportError);
@@ -58,6 +59,8 @@ export class AccountPage extends Disposable {
       headerMain: this._buildHeaderMain(),
       contentMain: this._buildContentMain(),
       testId,
+    }, {
+      regionFocusSwitcher: this._appObj.regionFocusSwitcher,
     });
   }
 

app/client/ui/AdminPanel.ts

@@ -4,6 +4,7 @@ import {localStorageJsonObs} from 'app/client/lib/localStorageObs';
 import {getTimeFromNow} from 'app/client/lib/timeUtils';
 import {AdminChecks, probeDetails, ProbeDetails} from 'app/client/models/AdminChecks';
 import {AppModel, getHomeUrl, reportError} from 'app/client/models/AppModel';
+import {App} from 'app/client/ui/App';
 import {AuditLogsModel} from 'app/client/models/AuditLogsModel';
 import {urlState} from 'app/client/models/gristUrlState';
 import {showEnterpriseToggle} from 'app/client/ui/ActivationPage';
@@ -35,7 +36,7 @@ const t = makeT('AdminPanel');
 export class AdminPanel extends Disposable {
   private _page = Computed.create<AdminPanelPage>(this, (use) => use(urlState().state).adminPanel || 'admin');
 
-  constructor(private _appModel: AppModel) {
+  constructor(private _appModel: AppModel, private _appObj: App) {
     super();
     document.title = getAdminPanelName() + getPageTitleSuffix(getGristConfig());
   }
@@ -46,6 +47,8 @@ export class AdminPanel extends Disposable {
       headerMain: this._buildMainHeader(),
       contentTop: buildHomeBanners(this._appModel),
       contentMain: this._buildMainContent(),
+    }, {
+      regionFocusSwitcher: this._appObj.regionFocusSwitcher,
     });
   }
 

app/client/ui/App.ts

@@ -1,5 +1,7 @@
 import {ClientScope} from 'app/client/components/ClientScope';
 import * as Clipboard from 'app/client/components/Clipboard';
+import {RegionFocusSwitcher} from 'app/client/components/RegionFocusSwitcher';
+import {KeyboardFocusHighlighter} from 'app/client/components/KeyboardFocusHighlighter';
 import {Comm} from 'app/client/components/Comm';
 import * as commandList from 'app/client/components/commandList';
 import * as commands from 'app/client/components/commands';
@@ -24,7 +26,6 @@ import {dom} from 'grainjs';
 import * as ko from 'knockout';
 import {makeT} from 'app/client/lib/localization';
 import { onClickOutside } from 'app/client/lib/domUtils';
-import {KeyboardFocusHighlighter} from 'app/client/components/KeyboardFocusHighlighter';
 
 const t = makeT('App');
 
@@ -39,6 +40,7 @@ export interface App extends DisposableWithEvents {
   features: ko.Computed<ISupportedFeatures>;
   topAppModel: TopAppModel;
   pageModel?: DocPageModel;
+  regionFocusSwitcher?: RegionFocusSwitcher;
 }
 
 /**
@@ -56,6 +58,8 @@ export class AppImpl extends DisposableWithEvents implements App {
   // Track the most recently created DocPageModel, for some error handling.
   public pageModel?: DocPageModel;
 
+  public regionFocusSwitcher?: RegionFocusSwitcher;
+
   private _settings: ko.Observable<{features?: ISupportedFeatures}>;
 
   // Track the version of the server we are communicating with, so that if it changes
@@ -76,9 +80,10 @@ export class AppImpl extends DisposableWithEvents implements App {
     this._settings = ko.observable({});
     this.features = ko.computed(() => this._settings().features || {});
 
-    this.autoDispose(new KeyboardFocusHighlighter());
+    KeyboardFocusHighlighter.create(this);
 
     if (isDesktop()) {
+      this.regionFocusSwitcher = RegionFocusSwitcher.create(this, this);
       this.autoDispose(Clipboard.create(this));
     } else {
       // On mobile, we do not want to keep focus on a special textarea (which would cause unwanted

app/client/ui/AppUI.ts

@@ -83,15 +83,15 @@ function createMainPage(appModel: AppModel, appObj: App) {
     } else if (pageType === 'billing') {
       return domAsync(loadBillingPage().then(bp => dom.create(bp.BillingPage, appModel)));
     } else if (pageType === 'welcome') {
-      return dom.create(WelcomePage, appModel);
+      return dom.create(WelcomePage, appModel, appObj);
     } else if (pageType === 'account') {
-      return domAsync(loadAccountPage().then(ap => dom.create(ap.AccountPage, appModel)));
+      return domAsync(loadAccountPage().then(ap => dom.create(ap.AccountPage, appModel, appObj)));
     } else if (pageType === 'admin') {
-      return domAsync(loadAdminPanel().then(m => dom.create(m.AdminPanel, appModel)));
+      return domAsync(loadAdminPanel().then(m => dom.create(m.AdminPanel, appModel, appObj)));
     } else if (pageType === 'activation') {
       return domAsync(loadActivationPage().then(ap => dom.create(ap.getActivationPage(), appModel)));
     } else if (pageType === 'audit-logs') {
-      return domAsync(loadAuditLogsPage().then(m => dom.create(m.AuditLogsPage, appModel)));
+      return domAsync(loadAuditLogsPage().then(m => dom.create(m.AuditLogsPage, appModel, appObj)));
     } else {
       return dom.create(pagePanelsDoc, appModel, appObj);
     }
@@ -128,6 +128,8 @@ function pagePanelsHome(owner: IDisposableOwner, appModel: AppModel, app: App) {
     contentMain: createDocMenu(pageModel),
     contentTop: buildHomeBanners(appModel),
     testId,
+  }, {
+    regionFocusSwitcher: app.regionFocusSwitcher,
   });
 }
 
@@ -185,6 +187,6 @@ function pagePanelsDoc(owner: IDisposableOwner, appModel: AppModel, appObj: App)
     contentBottom: dom.create(createBottomBarDoc, pageModel, leftPanelOpen, rightPanelOpen),
     banner: dom.create(ViewAsBanner, pageModel),
   }, {
-    gristDoc: pageModel.gristDoc,
+    regionFocusSwitcher: appObj.regionFocusSwitcher,
   });
 }

app/client/ui/AuditLogsPage.ts

@@ -1,6 +1,7 @@
 import { buildHomeBanners } from "app/client/components/Banners";
 import { makeT } from "app/client/lib/localization";
 import { AppModel, reportError } from "app/client/models/AppModel";
+import { App } from "app/client/ui/App";
 import { AuditLogsModel } from "app/client/models/AuditLogsModel";
 import { urlState } from "app/client/models/gristUrlState";
 import { AppHeader } from "app/client/ui/AppHeader";
@@ -40,7 +41,7 @@ export class AuditLogsPage extends Disposable {
     (_use, s) => s.auditLogs
   );
 
-  constructor(private _appModel: AppModel) {
+  constructor(private _appModel: AppModel, private _appObj: App) {
     super();
     this._setTitle();
   }
@@ -73,6 +74,8 @@ export class AuditLogsPage extends Disposable {
       headerMain: this._buildHeader(),
       contentTop: buildHomeBanners(this._appModel),
       contentMain: this._buildContent(),
+    }, {
+      regionFocusSwitcher: this._appObj.regionFocusSwitcher,
     });
   }
 

app/client/ui/PagePanels.ts

@@ -16,7 +16,6 @@ import once from 'lodash/once';
 import {SessionObs} from 'app/client/lib/sessionObs';
 import debounce from 'lodash/debounce';
 import {RegionFocusSwitcher} from 'app/client/components/RegionFocusSwitcher';
-import {GristDoc} from 'app/client/components/GristDoc';
 
 const t = makeT('PagePanels');
 
@@ -50,10 +49,7 @@ export interface PageContents {
 }
 
 interface PagePanelsOptions {
-  /**
-   * If provided, the region kb focus switcher will also cycle through the given doc's widgets.
-   */
-  gristDoc?: Observable<GristDoc | null>;
+  regionFocusSwitcher?: RegionFocusSwitcher;
 }
 
 export function pagePanels(
@@ -69,6 +65,8 @@ export function pagePanels(
   const bannerHeight = Observable.create(null, 0);
   const isScreenResizingObs = isScreenResizing();
 
+  const regionFocusSwitcher = options.regionFocusSwitcher;
+
   let lastLeftOpen = left.panelOpen.get();
   let lastRightOpen = right?.panelOpen.get() || false;
   let leftPaneDom: HTMLElement;
@@ -77,10 +75,6 @@ export function pagePanels(
   let contentTopDom: HTMLElement;
   let onLeftTransitionFinish = noop;
 
-  const regionFocusSwitcher = RegionFocusSwitcher.create(null, options.gristDoc);
-  // @ts-expect-error just dirty code to check an idea. To remove later.
-  window.gristRegionFocusSwitcher = regionFocusSwitcher;
-
   // When switching to mobile mode, close panels; when switching to desktop, restore the
   // last desktop state.
   const sub1 = subscribe(isNarrowScreenObs(), (use, narrow) => {
@@ -103,7 +97,7 @@ export function pagePanels(
     if (narrow) {
       left.panelOpen.set(false);
     }
-    regionFocusSwitcher.reset();
+    regionFocusSwitcher?.reset();
   });
 
   const pauseSavingLeft = (yesNo: boolean) => {
@@ -151,11 +145,11 @@ export function pagePanels(
     }),
     cssContentMain(
       (el) => {
-        regionFocusSwitcher.init(el);
+        regionFocusSwitcher?.init(el);
       },
       leftPaneDom = cssLeftPane(
         testId('left-panel'),
-        regionFocusSwitcher.panelAttrs('left', t('Main navigation and document settings (left panel)')),
+        regionFocusSwitcher?.panelAttrs('left', t('Main navigation and document settings (left panel)')),
         cssOverflowContainer(
           contentWrapper = cssLeftPanelContainer(
             cssLeftPaneHeader(
@@ -292,7 +286,7 @@ export function pagePanels(
       cssMainPane(
         mainHeaderDom = cssTopHeader(
           testId('top-header'),
-          regionFocusSwitcher.panelAttrs('top', t('Document header')),
+          regionFocusSwitcher?.panelAttrs('top', t('Document header')),
           (left.hideOpener ? null :
             cssPanelOpener('PanelRight', cssPanelOpener.cls('-open', left.panelOpen),
               testId('left-opener'),
@@ -315,7 +309,7 @@ export function pagePanels(
         ),
 
         cssContentMainPane(
-          regionFocusSwitcher.panelAttrs('main', t('Main content')),
+          regionFocusSwitcher?.panelAttrs('main', t('Main content')),
           page.contentMain,
         ),
 
@@ -332,7 +326,7 @@ export function pagePanels(
 
         rightPaneDom = cssRightPane(
           testId('right-panel'),
-          regionFocusSwitcher.panelAttrs('right', t('Creator panel (right panel)')),
+          regionFocusSwitcher?.panelAttrs('right', t('Creator panel (right panel)')),
           cssRightPaneHeader(
             right.header,
             dom.style('margin-bottom', use => use(bannerHeight) + 'px')

app/client/ui/WelcomePage.ts

@@ -2,6 +2,7 @@ import { Disposable, dom, domComputed, DomContents, MultiHolder, Observable, sty
 
 import { handleSubmit } from "app/client/lib/formUtils";
 import { AppModel } from "app/client/models/AppModel";
+import { App } from 'app/client/ui/App';
 import { getLoginUrl, getSignupUrl, urlState } from "app/client/models/gristUrlState";
 import { AccountWidget } from "app/client/ui/AccountWidget";
 import { AppHeader } from 'app/client/ui/AppHeader';
@@ -36,7 +37,7 @@ function handleSubmitForm(
 
 export class WelcomePage extends Disposable {
 
-  constructor(private _appModel: AppModel) {
+  constructor(private _appModel: AppModel, private _appObj: App) {
     super();
   }
 
@@ -58,6 +59,8 @@ export class WelcomePage extends Disposable {
         page === 'teams' ? dom.create(buildWelcomeSitePicker, this._appModel) :
         this._buildPageContent(page)
       ),
+    }, {
+      regionFocusSwitcher: this._appObj.regionFocusSwitcher,
     });
   }