Update shortcut dialog to use new ButtonDialog component

Bug: 379036632
Change-Id: I611a2b99febfa99043a4b42ef1141d6370ba8510
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6023184
Reviewed-by: Alina Varkki <[email protected]>
Reviewed-by: Kateryna Prokopenko <[email protected]>
Commit-Queue: Nancy Li <[email protected]>

Author: Nancy Li

front_end/ui/components/dialogs/ShortcutDialog.test.ts

@@ -27,29 +27,15 @@ describeWithLocale('ShortcutDialog', () => {
 
   function getDialogFromShortcutDialog(shortcutDialog: Dialogs.ShortcutDialog.ShortcutDialog) {
     assert.isNotNull(shortcutDialog.shadowRoot);
-    const dialog = shortcutDialog.shadowRoot.querySelector('devtools-dialog');
+    const dialog = shortcutDialog.shadowRoot.querySelector('devtools-button-dialog');
     if (!dialog) {
-      assert.fail('devtools-dialog not found');
+      assert.fail('devtools-button-dialog not found');
     }
     assert.instanceOf(dialog, HTMLElement);
     return dialog;
   }
 
-  it('should display dialog on initial render when provided prop', async () => {
-    const shortcutDialog = await getShortcutDialog(true);
-    const dialog = getDialogFromShortcutDialog(shortcutDialog);
-
-    assert.isTrue(dialog.hasAttribute('open'));
-  });
-
-  it('should not display dialog on initial render by default', async () => {
-    const shortcutDialog = await getShortcutDialog();
-    const dialog = getDialogFromShortcutDialog(shortcutDialog);
-
-    assert.isFalse(dialog.hasAttribute('open'));
-  });
-
-  it('prepends provived element to the dialog content', async () => {
+  it('prepends provided element to the dialog content', async () => {
     const prependedElement = document.createElement('div');
     prependedElement.classList.add('prepended-element');
 

front_end/ui/components/dialogs/ShortcutDialog.ts

@@ -1,20 +1,15 @@
 // Copyright 2023 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
+import './ButtonDialog.js';
 
 import * as i18n from '../../../core/i18n/i18n.js';
 import type * as Platform from '../../../core/platform/platform.js';
 import * as Buttons from '../../../ui/components/buttons/buttons.js';
 import * as ComponentHelpers from '../../../ui/components/helpers/helpers.js';
 import * as LitHtml from '../../../ui/lit-html/lit-html.js';
-import * as VisualLogging from '../../../ui/visual_logging/visual_logging.js';
-
-import {
-  type ClickOutsideDialogEvent,
-  type Dialog as DialogElement,
-  DialogHorizontalAlignment,
-  DialogVerticalPosition,
-} from './Dialog.js';
+
+import type {ButtonDialogData} from './ButtonDialog.js';
 import shortcutDialogStyles from './shortcutDialog.css.js';
 
 const {html} = LitHtml;
@@ -29,10 +24,6 @@ const UIStrings = {
    * @description Title of the keyboard shortcuts help menu.
    */
   dialogTitle: 'Keyboard shortcuts',
-  /**
-   * @description Title of close button for the shortcuts dialog.
-   */
-  close: 'Close',
 };
 
 const str_ = i18n.i18n.registerUIStrings('ui/components/dialogs/ShortcutDialog.ts', UIStrings);
@@ -44,14 +35,6 @@ declare global {
   }
 }
 
-export class ShowDialog extends Event {
-  static readonly eventName = 'showdialog';
-
-  constructor() {
-    super(ShowDialog.eventName);
-  }
-}
-
 export interface Shortcut {
   title: string|Platform.UIString.LocalizedString;
   bindings: string[];
@@ -65,8 +48,6 @@ export class ShortcutDialog extends HTMLElement {
   readonly #shadow = this.attachShadow({mode: 'open'});
   readonly #renderBound = this.#render.bind(this);
 
-  #dialog: DialogElement|null = null;
-  #showButton: Buttons.Button.Button|null = null;
   #shortcuts: Shortcut[] = [];
   #openOnRender = false;
   #prependedElement: HTMLElement|null = null;
@@ -84,26 +65,6 @@ export class ShortcutDialog extends HTMLElement {
     void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#renderBound);
   }
 
-  #showDialog(): void {
-    if (!this.#dialog) {
-      throw new Error('Dialog not found');
-    }
-    void this.#dialog.setDialogVisible(true);
-    void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#renderBound);
-    this.dispatchEvent(new ShowDialog());
-  }
-
-  #closeDialog(evt?: ClickOutsideDialogEvent): void {
-    if (!this.#dialog) {
-      throw new Error('Dialog not found');
-    }
-    void this.#dialog.setDialogVisible(false);
-    if (evt) {
-      evt.stopImmediatePropagation();
-    }
-    void ComponentHelpers.ScheduledRender.scheduleRender(this, this.#renderBound);
-  }
-
   #getKeysFromBinding(binding: string): string[] {
     return binding.split(/[\s+]+/).map(word => word.trim());  // Split on one or more spaces or + symbols
   }
@@ -120,47 +81,16 @@ export class ShortcutDialog extends HTMLElement {
     // clang-format off
     LitHtml.render(
       html`
-      <devtools-button
-        @click=${this.#showDialog}
-        on-render=${ComponentHelpers.Directives.nodeRenderedCallback(node => {
-          this.#showButton = node as Buttons.Button.Button;
-        })}
-        .data=${{
+      <devtools-button-dialog .data=${{
+          openOnRender: this.#openOnRender,
+          closeButton: true,
+          dialogTitle: i18nString(UIStrings.dialogTitle),
           variant: Buttons.Button.Variant.TOOLBAR,
           iconName: 'help',
-          title: i18nString(UIStrings.showShortcutTitle),
-        } as Buttons.Button.ButtonData}
-      ></devtools-button>
-      <devtools-dialog
-        class="shortcuts-dialog"
-        @clickoutsidedialog=${this.#closeDialog}
-        .origin=${() => {
-          if (!this.#showButton) {
-            throw new Error('Button not found');
-          }
-          return this.#showButton;
-        }}
-        .position=${DialogVerticalPosition.BOTTOM}
-        .horizontalAlignment=${DialogHorizontalAlignment.RIGHT}
-        .jslogContext=${'shortcuts'}
-        on-render=${ComponentHelpers.Directives.nodeRenderedCallback(node => {
-          this.#dialog = node as DialogElement;
-        })}
-      >
-      <div class="keybinds-category-header">
-        <span class="keybinds-category-header-text">${i18nString(UIStrings.dialogTitle)}</span>
-        <devtools-button
-        @click=${this.#closeDialog}
-        .data=${{
-          variant: Buttons.Button.Variant.TOOLBAR,
-          iconName: 'cross',
-          title: i18nString(UIStrings.close),
-        } as Buttons.Button.ButtonData}
-            jslog=${VisualLogging.close().track({click: true})}
-            ></devtools-button>
-          </div>
-        ${(this.#prependedElement) ? html`${this.#prependedElement}` : LitHtml.nothing}
+          iconTitle: i18nString(UIStrings.showShortcutTitle),
+        } as ButtonDialogData}>
         <ul class="keybinds-list">
+          ${(this.#prependedElement) ? html`${this.#prependedElement}` : LitHtml.nothing}
           ${this.#shortcuts.map(shortcut =>
             html`
               <li class="keybinds-list-item">
@@ -179,15 +109,10 @@ export class ShortcutDialog extends HTMLElement {
               </li>`,
           )}
         </ul>
-      </devtools-dialog>
+      </devtools-button-dialog>
       `,
       this.#shadow, {host: this});
     // clang-format on
-
-    if (this.#openOnRender) {
-      this.#showDialog();
-      this.#openOnRender = false;
-    }
   }
 }
 

front_end/ui/components/dialogs/shortcutDialog.css

@@ -4,11 +4,6 @@
  * found in the LICENSE file.
  */
 
-.shortcuts-dialog {
-  /* overwrite 'white-space: pre' in the toolbar item parent element */
-  white-space: nowrap;
-}
-
 .keybinds-category-header {
   display: flex;
   justify-content: space-between;
@@ -24,8 +19,9 @@
 .keybinds-list {
   display: flex;
   flex-direction: column;
+  /* overwrite default`margin` and `padding` for the <ul> element */
   margin: 0;
-  padding: 0 var(--sys-size-8) var(--sys-size-3) var(--sys-size-8);
+  padding: 0;
 }
 
 .keybinds-list-item {

front_end/ui/components/docs/BUILD.gn

@@ -53,7 +53,6 @@ group("docs") {
     "./report",
     "./request_link_icon",
     "./select_menu",
-    "./shortcut_dialog",
     "./style_property_editor",
     "./survey_link",
     "./switch",

front_end/ui/components/docs/dialog/BUILD.gn

@@ -10,6 +10,7 @@ ts_library("ts") {
   sources = [
     "basic.ts",
     "button_dialog.ts",
+    "shortcut_dialog.ts",
   ]
 
   deps = [
@@ -24,6 +25,7 @@ copy_to_gen("dialog") {
   sources = [
     "basic.html",
     "button_dialog.html",
+    "shortcut_dialog.html",
   ]
   deps = [ ":ts" ]
 }

…mponents/docs/shortcut_dialog/basic.html …ponents/docs/dialog/shortcut_dialog.htmlfront_end/ui/components/docs/shortcut_dialog/basic.html renamed to front_end/ui/components/docs/dialog/shortcut_dialog.html front_end/ui/components/docs/shortcut_dialog/basic.html renamed to front_end/ui/components/docs/dialog/shortcut_dialog.html

@@ -23,6 +23,6 @@
 
   <body>
     <div id="container"></div>
-    <script src="./basic.js" type="module"></script>
+    <script src="./shortcut_dialog.js" type="module"></script>
   </body>
 </html>

…components/docs/shortcut_dialog/basic.ts …omponents/docs/dialog/shortcut_dialog.tsfront_end/ui/components/docs/shortcut_dialog/basic.ts renamed to front_end/ui/components/docs/dialog/shortcut_dialog.ts front_end/ui/components/docs/shortcut_dialog/basic.ts renamed to front_end/ui/components/docs/dialog/shortcut_dialog.ts

@@ -2,9 +2,9 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import * as ComponentHelpers from '../../../../../front_end/ui/components/helpers/helpers.js';
 import * as FrontendHelpers from '../../../../testing/EnvironmentHelpers.js';  // eslint-disable-line rulesdir/es_modules_import
-import * as Dialogs from '../../../../ui/components/dialogs/dialogs.js';
+import * as Dialogs from '../../dialogs/dialogs.js';
+import * as ComponentHelpers from '../../helpers/helpers.js';
 
 await ComponentHelpers.ComponentServerSetup.setup();
 await FrontendHelpers.initializeGlobalVars();

front_end/ui/components/docs/shortcut_dialog/BUILD.gn

@@ -329,13 +329,14 @@ describe('Recorder', function() {
         const {frontend} = getBrowserAndPages();
         await frontend.bringToFront();
         const shortcutDialog = await waitFor('devtools-shortcut-dialog');
+        const buttonDialog = await waitFor('devtools-button-dialog', shortcutDialog);
 
-        await click('devtools-button', {root: shortcutDialog});
+        await click('devtools-button', {root: buttonDialog});
 
-        const dialog = await waitFor('devtools-dialog', shortcutDialog);
+        const dialog = await waitFor('devtools-dialog', buttonDialog);
         assert.isOk(dialog);
 
-        const shortcuts = await $$('.keybinds-list-item', dialog);
+        const shortcuts = await $$('.keybinds-list-item', buttonDialog);
         assert.lengthOf(shortcuts, 4);
       });
     });