Welcome: Change NavigationBehavior to NavigationMixin.

Behaviors are deprecated when using class-based Polymer syntax, and only supported as legacy code. The recommended way is to use Mixins instead, which also playing along nicer with TypeScript. Bug: 1189595 Change-Id: Ib3565b6ffb02af91154c08a82bb279ce6dcb8f8c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2880075 Commit-Queue: John Lee <johntlee@chromium.org> Auto-Submit: dpapad <dpapad@chromium.org> Reviewed-by: John Lee <johntlee@chromium.org> Cr-Commit-Position: refs/heads/master@{#881061}
chromium · May 10, 2021 · 253edc1 · 253edc1
1 parent fc3046d
commit 253edc1
Show file tree

Hide file tree

Showing 11 changed files with 141 additions and 129 deletions.
diff --git a/chrome/browser/resources/welcome/google_apps/nux_google_apps.ts b/chrome/browser/resources/welcome/google_apps/nux_google_apps.ts
@@ -19,7 +19,7 @@ import {isRTL} from 'chrome://resources/js/util.m.js';
 import {IronA11yAnnouncer} from 'chrome://resources/polymer/v3_0/iron-a11y-announcer/iron-a11y-announcer.js';
 import {afterNextRender, html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {navigateToNextStep, NavigationBehavior, NavigationBehaviorInterface} from '../navigation_behavior.js';
+import {navigateToNextStep, NavigationMixin, NavigationMixinInterface} from '../navigation_behavior.js';
 import {BookmarkBarManager, BookmarkProxy, BookmarkProxyImpl} from '../shared/bookmark_proxy.js';
 import {ModuleMetricsManager} from '../shared/module_metrics_proxy.js';
 import {stepIndicatorModel} from '../shared/nux_types.js';
@@ -44,10 +44,8 @@ type AppItemModel = {
 const KEYBOARD_FOCUSED = 'keyboard-focused';
 
 const NuxGoogleAppsElementBase =
-    mixinBehaviors([I18nBehavior, NavigationBehavior], PolymerElement) as {
-      new ():
-          PolymerElement & NavigationBehaviorInterface & I18nBehaviorInterface
-    };
+    mixinBehaviors([I18nBehavior], NavigationMixin(PolymerElement)) as
+    {new (): PolymerElement & NavigationMixinInterface & I18nBehaviorInterface};
 
 /** @polymer */
 export class NuxGoogleAppsElement extends NuxGoogleAppsElementBase {

diff --git a/chrome/browser/resources/welcome/landing_view.ts b/chrome/browser/resources/welcome/landing_view.ts
@@ -10,10 +10,10 @@ import './shared/splash_pages_shared_css.js';
 import '../strings.m.js';
 
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
-import {html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 import {LandingViewProxy, LandingViewProxyImpl} from './landing_view_proxy.js';
-import {navigateTo, NavigationBehavior, NavigationBehaviorInterface, Routes} from './navigation_behavior.js';
+import {navigateTo, NavigationMixin, Routes} from './navigation_behavior.js';
 import {OnboardingBackgroundElement} from './shared/onboarding_background.js';
 import {WelcomeBrowserProxyImpl} from './welcome_browser_proxy.js';
 
@@ -23,9 +23,7 @@ export interface LandingViewElement {
   };
 }
 
-const LandingViewElementBase =
-    mixinBehaviors([NavigationBehavior], PolymerElement) as
-    {new (): PolymerElement & NavigationBehaviorInterface};
+const LandingViewElementBase = NavigationMixin(PolymerElement);
 
 /** @polymer */
 export class LandingViewElement extends LandingViewElementBase {

diff --git a/chrome/browser/resources/welcome/navigation_behavior.ts b/chrome/browser/resources/welcome/navigation_behavior.ts
@@ -6,10 +6,10 @@ import '../strings.m.js';
 
 import {assert} from 'chrome://resources/js/assert.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
-import {afterNextRender} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+import {afterNextRender, dedupingMixin, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
 /**
- * @fileoverview The NavigationBehavior is in charge of manipulating and
+ * @fileoverview The NavigationMixin is in charge of manipulating and
  *     watching window.history.state changes. The page is using the history
  *     state object to remember state instead of changing the URL directly,
  *     because the flow requires that users can use browser-back/forward to
@@ -50,10 +50,10 @@ if (!history.state || !history.state.route || !history.state.step) {
   }
 }
 
-const routeObservers: Set<NavigationBehaviorInterface> = new Set();
-let currentRouteElement: NavigationBehaviorInterface|null;
+const routeObservers: Set<NavigationMixinInterface> = new Set();
+let currentRouteElement: NavigationMixinInterface|null;
 
-// Notifies all the elements that extended NavigationBehavior.
+// Notifies all the elements that extended NavigationMixin.
 function notifyObservers() {
   if (currentRouteElement) {
     currentRouteElement.onRouteExit();
@@ -74,7 +74,7 @@ function notifyObservers() {
 
   // If currentRouteElement is not null, it means there was a new route.
   if (currentRouteElement) {
-    (currentRouteElement as NavigationBehaviorInterface).notifyRouteEnter();
+    (currentRouteElement as NavigationMixinInterface).notifyRouteEnter();
   }
 }
 
@@ -111,79 +111,90 @@ export function navigateTo(route: Routes, step: number) {
   notifyObservers();
 }
 
+type Constructor<T> = new (...args: any[]) => T;
+
 /**
  * Elements can override onRoute(Change|Enter|Exit) to handle route changes.
  * Order of hooks being called:
  *   1) onRouteExit() on the old route
  *   2) onRouteChange() on all subscribed routes
  *   3) onRouteEnter() on the new route
- *
- * @polymerBehavior
  */
-export const NavigationBehavior = {
-  subtitle: '',
-
-  attached() {
-    assert(!routeObservers.has(this));
-    routeObservers.add(this);
-    const route = (history.state.route as Routes);
-    const step = history.state.step;
-
-    // history state was set when page loaded, so when the element first
-    // attaches, call the route-change handler to initialize first.
-    this.onRouteChange(route, step);
-
-    // Modules are only attached to DOM if they're for the current route, so
-    // as long as the id of an element matches up to the current step, it
-    // means that element is for the current route.
-    // TODO(crbug.com/1189595): Figure out the proper way of making TS
-    // understand that this class is an HTMLElement.
-    if ((this as any).id === `step-${step}`) {
-      currentRouteElement = this;
-      this.notifyRouteEnter();
-    }
-  },
-
-  /**
-   * Notifies elements that route was entered and updates the state of the
-   * app based on the new route.
-   */
-  notifyRouteEnter(): void {
-    this.onRouteEnter();
-    this.updateFocusForA11y();
-    this.updateTitle();
-  },
-
-  /** Called to update focus when progressing through the modules. */
-  updateFocusForA11y(): void {
-    // TODO(crbug.com/1189595): Figure out the proper way of making TS
-    // understand that this class is also a LegacyElementMixin
-    const header = (this as any).$$('h1');
-    if (header) {
-      afterNextRender(this, () => header.focus());
-    }
-  },
-
-  updateTitle(): void {
-    let title = loadTimeData.getString('headerText');
-    if (this.subtitle) {
-      title += ' - ' + this.subtitle;
-    }
-    document.title = title;
-  },
-
-  detached() {
-    assert(routeObservers.delete(this));
-  },
-
-  onRouteChange(_route: Routes, _step: number): void{},
-  onRouteEnter(): void{},
-  onRouteExit(): void{},
-  onRouteUnload(): void {}
-};
-
-export interface NavigationBehaviorInterface {
-  subtitle: string;
+export const NavigationMixin = dedupingMixin(
+    <T extends Constructor<PolymerElement>>(superClass: T): T&
+    Constructor<NavigationMixinInterface> => {
+      class NavigationMixin extends superClass {
+        static get properties() {
+          return {
+            subtitle: String,
+          };
+        }
+
+        subtitle?: string;
+
+        connectedCallback() {
+          super.connectedCallback();
+
+          assert(!routeObservers.has(this));
+          routeObservers.add(this);
+          const route = (history.state.route as Routes);
+          const step = history.state.step;
+
+          // history state was set when page loaded, so when the element first
+          // attaches, call the route-change handler to initialize first.
+          this.onRouteChange(route, step);
+
+          // Modules are only attached to DOM if they're for the current route,
+          // so as long as the id of an element matches up to the current step,
+          // it means that element is for the current route.
+          if (this.id === `step-${step}`) {
+            currentRouteElement = this;
+            this.notifyRouteEnter();
+          }
+        }
+
+        /**
+         * Notifies elements that route was entered and updates the state of the
+         * app based on the new route.
+         */
+        notifyRouteEnter(): void {
+          this.onRouteEnter();
+          this.updateFocusForA11y();
+          this.updateTitle();
+        }
+
+        /** Called to update focus when progressing through the modules. */
+        updateFocusForA11y(): void {
+          const header = this.shadowRoot!.querySelector('h1');
+          if (header) {
+            afterNextRender(this, () => header.focus());
+          }
+        }
+
+        updateTitle(): void {
+          let title = loadTimeData.getString('headerText');
+          if (this.subtitle) {
+            title += ' - ' + this.subtitle;
+          }
+          document.title = title;
+        }
+
+        disconnectedCallback() {
+          super.disconnectedCallback();
+          assert(routeObservers.delete(this));
+        }
+
+        onRouteChange(_route: Routes, _step: number): void {}
+        onRouteEnter(): void {}
+        onRouteExit(): void {}
+        onRouteUnload(): void {}
+      }
+
+      return NavigationMixin;
+    });
+
+export interface NavigationMixinInterface {
+  subtitle?: string;
   notifyRouteEnter(): void;
   updateFocusForA11y(): void;
   updateTitle(): void;

diff --git a/chrome/browser/resources/welcome/ntp_background/nux_ntp_background.ts b/chrome/browser/resources/welcome/ntp_background/nux_ntp_background.ts
@@ -17,7 +17,7 @@ import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {isRTL} from 'chrome://resources/js/util.m.js';
 import {html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {navigateToNextStep, NavigationBehavior, NavigationBehaviorInterface} from '../navigation_behavior.js';
+import {navigateToNextStep, NavigationMixin, NavigationMixinInterface} from '../navigation_behavior.js';
 import {ModuleMetricsManager} from '../shared/module_metrics_proxy.js';
 import {stepIndicatorModel} from '../shared/nux_types.js';
 
@@ -33,10 +33,8 @@ export interface NuxNtpBackgroundElement {
 }
 
 const NuxNtpBackgroundElementBase =
-    mixinBehaviors([I18nBehavior, NavigationBehavior], PolymerElement) as {
-      new ():
-          PolymerElement & NavigationBehaviorInterface & I18nBehaviorInterface
-    };
+    mixinBehaviors([I18nBehavior], NavigationMixin(PolymerElement)) as
+    {new (): PolymerElement & NavigationMixinInterface & I18nBehaviorInterface};
 
 /** @polymer */
 export class NuxNtpBackgroundElement extends NuxNtpBackgroundElementBase {

diff --git a/chrome/browser/resources/welcome/set_as_default/nux_set_as_default.ts b/chrome/browser/resources/welcome/set_as_default/nux_set_as_default.ts
@@ -16,16 +16,16 @@ import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
 import {WebUIListenerBehavior, WebUIListenerBehaviorInterface} from 'chrome://resources/js/web_ui_listener_behavior.m.js';
 import {html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {navigateToNextStep, NavigationBehavior, NavigationBehaviorInterface} from '../navigation_behavior.js';
+import {navigateToNextStep, NavigationMixin, NavigationMixinInterface} from '../navigation_behavior.js';
 import {DefaultBrowserInfo, stepIndicatorModel} from '../shared/nux_types.js';
 
 import {NuxSetAsDefaultProxy, NuxSetAsDefaultProxyImpl} from './nux_set_as_default_proxy.js';
 
 const NuxSetAsDefaultElementBase =
-    mixinBehaviors(
-        [WebUIListenerBehavior, NavigationBehavior], PolymerElement) as {
+    mixinBehaviors([WebUIListenerBehavior], NavigationMixin(PolymerElement)) as
+    {
       new (): PolymerElement & WebUIListenerBehaviorInterface &
-      NavigationBehaviorInterface
+      NavigationMixinInterface
     };
 
 /** @polymer */

diff --git a/chrome/browser/resources/welcome/signin_view.ts b/chrome/browser/resources/welcome/signin_view.ts
@@ -10,9 +10,9 @@ import './shared/onboarding_background.js';
 import './shared/splash_pages_shared_css.js';
 import '../strings.m.js';
 
-import {html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {NavigationBehavior, NavigationBehaviorInterface} from './navigation_behavior.js';
+import {NavigationMixin} from './navigation_behavior.js';
 import {OnboardingBackgroundElement} from './shared/onboarding_background.js';
 import {SigninViewProxy, SigninViewProxyImpl} from './signin_view_proxy.js';
 import {WelcomeBrowserProxy, WelcomeBrowserProxyImpl} from './welcome_browser_proxy.js';
@@ -23,9 +23,7 @@ export interface SigninViewElement {
   };
 }
 
-const SigninViewElementBase =
-    mixinBehaviors([NavigationBehavior], PolymerElement) as
-    {new (): PolymerElement & NavigationBehaviorInterface};
+const SigninViewElementBase = NavigationMixin(PolymerElement);
 
 /** @polymer */
 export class SigninViewElement extends SigninViewElementBase {

diff --git a/chrome/browser/resources/welcome/welcome_app.ts b/chrome/browser/resources/welcome/welcome_app.ts
@@ -15,9 +15,9 @@ import '../strings.m.js';
 import {CrViewManagerElement} from 'chrome://resources/cr_elements/cr_view_manager/cr_view_manager.js';
 import {assert} from 'chrome://resources/js/assert.m.js';
 import {loadTimeData} from 'chrome://resources/js/load_time_data.m.js';
-import {html, mixinBehaviors, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
+import {html, PolymerElement} from 'chrome://resources/polymer/v3_0/polymer/polymer_bundled.min.js';
 
-import {NavigationBehavior, NavigationBehaviorInterface, Routes} from './navigation_behavior.js';
+import {NavigationMixin, Routes} from './navigation_behavior.js';
 import {NuxSetAsDefaultProxyImpl} from './set_as_default/nux_set_as_default_proxy.js';
 import {BookmarkBarManager} from './shared/bookmark_proxy.js';
 import {WelcomeBrowserProxyImpl} from './welcome_browser_proxy.js';
@@ -51,9 +51,7 @@ export interface WelcomeAppElement {
   };
 }
 
-const WelcomeAppElementBase =
-    mixinBehaviors([NavigationBehavior], PolymerElement) as
-    {new (): PolymerElement & NavigationBehaviorInterface};
+const WelcomeAppElementBase = NavigationMixin(PolymerElement);
 
 /** @polymer */
 export class WelcomeAppElement extends WelcomeAppElementBase {

diff --git a/chrome/test/data/webui/welcome/a11y_tests.js b/chrome/test/data/webui/welcome/a11y_tests.js
@@ -55,8 +55,8 @@ AccessibilityTest.define('WelcomeA11y', {
       assertEquals(
           'Make Chrome your own',
           document.body.querySelector('welcome-app')
-              .$$('landing-view')
-              .$$('h1')
+              .shadowRoot.querySelector('landing-view')
+              .shadowRoot.querySelector('h1')
               .textContent);
     },
   },