Remove Modal Tertiary Icon (#632)

CrowdStrike · Jan 24, 2025 · d31dfdc · d31dfdc
1 parent 42ff725
commit d31dfdc
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 168 deletions.
diff --git a/.changeset/real-bottles-play.md b/.changeset/real-bottles-play.md
@@ -0,0 +1,7 @@
+---
+'@crowdstrike/glide-core': minor
+---
+
+Modal Tertiary Icon has been removed.
+It was only a thin wrapper around Tooltip.
+Tooltip can be used directly instead.
diff --git a/src/modal.stories.ts b/src/modal.stories.ts
@@ -1,6 +1,5 @@
 import './button.js';
 import './icons/storybook.js';
-import './modal.tertiary-icon.js';
 import './tooltip.js';
 import { UPDATE_STORY_ARGS } from '@storybook/core-events';
 import { addons } from '@storybook/preview-api';
@@ -9,13 +8,28 @@ import { unsafeHTML } from 'lit/directives/unsafe-html.js';
 import { withActions } from '@storybook/addon-actions/decorator';
 import type { Meta, StoryObj } from '@storybook/web-components';
 import GlideCoreModal from './modal.js';
+import focusOutline from './styles/focus-outline.js';
 
 const meta: Meta = {
   title: 'Modal',
   decorators: [
     withActions,
     (story) => {
       return html`
+        <style>
+          [slot="target"] {
+            background-color: transparent;
+            border: none;
+            border-radius: 50%;
+            outline-offset: 1px;
+            padding: 0;
+
+            &:focus-visible {
+              ${focusOutline};
+            }
+          }
+        </style>
+
         ${story()}
         <glide-core-button label="Open"></glide-core-button>
       `;
@@ -76,14 +90,6 @@ const meta: Meta = {
         ?open=${arguments_.open}
       >
         ${unsafeHTML(arguments_['slot="default"'])}
-
-        <glide-core-button label="Primary" slot="primary"></glide-core-button>
-
-        <glide-core-button
-          label="Secondary"
-          slot="secondary"
-          variant="tertiary"
-        ></glide-core-button>
       </glide-core-modal>
     `;
   },
@@ -102,10 +108,6 @@ const meta: Meta = {
     '<glide-core-modal-icon-button>.label': 'Edit',
     '<glide-core-modal-icon-button>[slot="default"]': '',
     '<glide-core-modal-icon-button>.version': '',
-    '<glide-core-modal-tertiary-icon>.label': 'Information',
-    '<glide-core-modal-tertiary-icon>[slot="default"]': '',
-    '<glide-core-modal-tertiary-icon>.tooltip-placement': 'bottom',
-    '<glide-core-modal-tertiary-icon>.version': '',
   },
   argTypes: {
     label: {
@@ -177,7 +179,7 @@ const meta: Meta = {
       control: false,
       table: {
         type: {
-          summary: 'GlideCoreButton | GlideCoreModalTertiaryIcon',
+          summary: 'GlideCoreButton | GlideCoreTooltip',
         },
       },
     },
@@ -218,48 +220,6 @@ const meta: Meta = {
         type: { summary: 'string', detail: '// For debugging' },
       },
     },
-    '<glide-core-modal-tertiary-icon>.label': {
-      name: 'label',
-      table: {
-        category: 'Modal Tertiary Icon',
-        type: { summary: 'string' },
-      },
-      type: { name: 'string', required: true },
-    },
-    '<glide-core-modal-tertiary-icon>[slot="default"]': {
-      name: 'slot="default"',
-      control: false,
-      table: {
-        category: 'Modal Tertiary Icon',
-        type: { summary: 'Element', detail: 'An icon' },
-      },
-      type: { name: 'string', required: true },
-    },
-    '<glide-core-modal-tertiary-icon>.tooltip-placement': {
-      name: 'tooltip-placement',
-      control: { type: 'radio' },
-      options: ['top', 'right', 'bottom', 'left'],
-      table: {
-        category: 'Modal Tertiary Icon',
-        defaultValue: { summary: '"bottom"' },
-        type: {
-          summary: '"top" | "right" | "bottom" | "left"',
-          detail:
-            '// The tooltip will try to move itself to the opposite of this value if not doing so would result in\n// overflow. For example, if "bottom" results in overflow the tooltip will try "top" but not "right"\n// or "left".',
-        },
-      },
-    },
-    '<glide-core-modal-tertiary-icon>.version': {
-      control: false,
-      name: 'version',
-      table: {
-        category: 'Modal Tertiary Icon',
-        defaultValue: {
-          summary: import.meta.env.VITE_CORE_VERSION,
-        },
-        type: { summary: 'string', detail: '// For debugging' },
-      },
-    },
   },
 };
 
@@ -284,13 +244,6 @@ export const WithHeaderActions: StoryObj = {
       >
         ${arguments_['slot="default"']}
 
-        <glide-core-button
-          label="Secondary"
-          slot="secondary"
-          variant="tertiary"
-        ></glide-core-button>
-
-        <!-- Only "glide-core-modal-icon-button" components should be used with header-actions -->
         <glide-core-modal-icon-button
           slot="header-actions"
           label=${arguments_['<glide-core-modal-icon-button>.label']}
@@ -306,7 +259,7 @@ export const WithHeaderActions: StoryObj = {
   },
 };
 
-export const WithTertiaryButton: StoryObj = {
+export const WithPrimaryButton: StoryObj = {
   render(arguments_) {
     return html`
       <script type="ignore">
@@ -321,12 +274,54 @@ export const WithTertiaryButton: StoryObj = {
         ${arguments_['slot="default"']}
 
         <glide-core-button label="Primary" slot="primary"></glide-core-button>
+      </glide-core-modal>
+    `;
+  },
+};
+
+export const WithSecondaryButton: StoryObj = {
+  render(arguments_) {
+    return html`
+      <script type="ignore">
+        import '@crowdstrike/glide-core/modal.js';
+      </script>
+
+      <glide-core-modal
+        label=${arguments_.label || nothing}
+        ?back-button="${arguments_['back-button']}"
+        ?open=${arguments_.open}
+      >
+        ${arguments_['slot="default"']}
 
         <glide-core-button
           label="Secondary"
           slot="secondary"
           variant="tertiary"
         ></glide-core-button>
+      </glide-core-modal>
+    `;
+  },
+};
+
+export const WithTertiaryTooltipAndButton: StoryObj = {
+  render(arguments_) {
+    return html`
+      <script type="ignore">
+        import '@crowdstrike/glide-core/modal.js';
+      </script>
+
+      <glide-core-modal
+        label=${arguments_.label || nothing}
+        ?back-button="${arguments_['back-button']}"
+        ?open=${arguments_.open}
+      >
+        ${arguments_['slot="default"']}
+
+        <glide-core-tooltip label="Tooltip" slot="tertiary" screenreader-hidden>
+          <button aria-label="Tooltip:" slot="target">
+            <glide-core-example-icon name="info"></glide-core-example-icon>
+          </button>
+        </glide-core-tooltip>
 
         <glide-core-button
           label="Tertiary"
@@ -338,12 +333,11 @@ export const WithTertiaryButton: StoryObj = {
   },
 };
 
-export const WithTertiaryIconAndButton: StoryObj = {
+export const KitchenSink: StoryObj = {
   render(arguments_) {
     return html`
       <script type="ignore">
         import '@crowdstrike/glide-core/modal.js';
-        import '@crowdstrike/glide-core/modal.tertiary-icon.js';
       </script>
 
       <glide-core-modal
@@ -353,29 +347,36 @@ export const WithTertiaryIconAndButton: StoryObj = {
       >
         ${arguments_['slot="default"']}
 
-        <glide-core-button label="Primary" slot="primary"></glide-core-button>
+        <glide-core-modal-icon-button
+          slot="header-actions"
+          label=${arguments_['<glide-core-modal-icon-button>.label']}
+        >
+          <glide-core-example-icon name="edit"></glide-core-example-icon>
+        </glide-core-modal-icon-button>
 
-        <glide-core-button
-          label="Secondary"
-          slot="secondary"
-          variant="tertiary"
-        ></glide-core-button>
+        <glide-core-modal-icon-button slot="header-actions" label="Settings">
+          <glide-core-example-icon name="settings"></glide-core-example-icon>
+        </glide-core-modal-icon-button>
 
-        <glide-core-modal-tertiary-icon
-          label=${arguments_['<glide-core-modal-tertiary-icon>.label']}
-          slot="tertiary"
-          tooltip-placement=${arguments_[
-            '<glide-core-modal-tertiary-icon>.tooltip-placement'
-          ]}
-        >
-          <glide-core-example-icon name="info"></glide-core-example-icon>
-        </glide-core-modal-tertiary-icon>
+        <glide-core-tooltip label="Tooltip" slot="tertiary" screenreader-hidden>
+          <button aria-label="Tooltip:" slot="target">
+            <glide-core-example-icon name="info"></glide-core-example-icon>
+          </button>
+        </glide-core-tooltip>
 
         <glide-core-button
           label="Tertiary"
           slot="tertiary"
           variant="tertiary"
         ></glide-core-button>
+
+        <glide-core-button
+          label="Secondary"
+          slot="secondary"
+          variant="tertiary"
+        ></glide-core-button>
+
+        <glide-core-button label="Primary" slot="primary"></glide-core-button>
       </glide-core-modal>
     `;
   },

diff --git a/src/modal.tertiary-icon.test.basics.ts b/src/modal.tertiary-icon.test.basics.ts
diff --git a/src/modal.tertiary-icon.ts b/src/modal.tertiary-icon.ts
diff --git a/src/modal.ts b/src/modal.ts
@@ -8,7 +8,7 @@ import packageJson from '../package.json' with { type: 'json' };
 import { LocalizeController } from './library/localize.js';
 import GlideCoreModalIconButton from './modal.icon-button.js';
 import GlideCoreButton from './button.js';
-import GlideCoreModalTertiaryIcon from './modal.tertiary-icon.js';
+import GlideCoreTooltip from './tooltip.js';
 import styles from './modal.styles.js';
 import xIcon from './icons/x.js';
 import assertSlot from './library/assert-slot.js';
@@ -36,7 +36,7 @@ globalStylesheet.insertRule(`
  * @slot header-actions - One or more of `<glide-core-modal-icon-button>`.
  * @slot primary - One of `<glide-core-button>`.
  * @slot secondary - One of `<glide-core-button>`.
- * @slot tertiary - One or more of `<glide-core-button>` or `<glide-core-modal-tertiary-icon>`.
+ * @slot tertiary - One or more of `<glide-core-button>` or `<glide-core-tooltip>`.
  */
 @customElement('glide-core-modal')
 export default class GlideCoreModal extends LitElement {
@@ -229,10 +229,7 @@ export default class GlideCoreModal extends LitElement {
               <slot
                 class="tertiary-slot"
                 name="tertiary"
-                ${assertSlot(
-                  [GlideCoreModalTertiaryIcon, GlideCoreButton],
-                  true,
-                )}
+                ${assertSlot([GlideCoreButton, GlideCoreTooltip], true)}
               ></slot>
             </li>
 

diff --git a/src/popover.stories.ts b/src/popover.stories.ts
@@ -37,7 +37,6 @@ const meta: Meta = {
             background-color: transparent;
             border: none;
             border-radius: 50%;
-            display: inline-flex;
             outline-offset: 1px;
             padding: 0;
 

diff --git a/src/tooltip.stories.ts b/src/tooltip.stories.ts
@@ -37,7 +37,6 @@ const meta: Meta = {
             background-color: transparent;
             border: none;
             border-radius: 50%;
-            display: inline-flex;
             outline-offset: 1px;
             padding: 0;