feat: theme fontawesome icon size wrapped in spectrum icons (#1658)

Spectrum icons are 18px. However, our icons look best at 16px as they were designed on a 16px grid. This overrides the default spectrum icon size when wrapping a font-awesome icon inside a spectrum icon wrapper. Made overrides not a module, so it doesn't add hashes to the classes in overrides, as I have a feeling this file will be used to target specific classes and that wouldn't be desirable. Note: please suggest somewhere for that variable I added to live if you think that location sucks
deephaven · Nov 27, 2023 · 2aa8cef · 2aa8cef
1 parent 89f2be5
commit 2aa8cef
Show file tree

Hide file tree

Showing 10 changed files with 109 additions and 95 deletions.
diff --git a/packages/code-studio/src/styleguide/SpectrumComponents.tsx b/packages/code-studio/src/styleguide/SpectrumComponents.tsx
@@ -31,7 +31,7 @@ import {
   Well,
 } from '@adobe/react-spectrum';
 import { FontAwesomeIcon } from '@fortawesome/react-fontawesome';
-import { dh } from '@deephaven/icons';
+import { dhTruck, vsEmptyWindow } from '@deephaven/icons';
 import { SPECTRUM_COMPONENT_SAMPLES_ID } from './constants';
 import { sampleSectionIdAndClassesSpectrum } from './utils';
 
@@ -75,78 +75,86 @@ export default SpectrumComponents;
 
 function ButtonsSample(): JSX.Element {
   return (
-    <Grid
-      autoFlow="column"
-      columnGap="size-250"
-      rowGap="size-150"
-      columns={repeat(4, 'size-2000')}
-      rows={repeat(8, 'size-400')}
-    >
-      <label>Button style=&quot;outline&quot;</label>
-      <Button variant="primary" style="outline">
-        Primary
-      </Button>
-      <Button variant="secondary" style="outline">
-        Secondary
-      </Button>
-      <Button variant="accent" style="outline">
-        Accent
-      </Button>
-      <Button variant="negative" style="outline">
-        Negative
-      </Button>
-      <Button variant="primary" staticColor="black" style="outline">
-        Static Black
-      </Button>
-      <Button variant="primary" staticColor="white" style="outline">
-        Static White
-      </Button>
-      <Button variant="primary" isDisabled style="outline">
-        Disabled
-      </Button>
+    <>
+      <ActionButton marginBottom="size-200">
+        <Icon>
+          <FontAwesomeIcon icon={dhTruck} />
+        </Icon>
+        <Text>Icon Button</Text>
+      </ActionButton>
+      <Grid
+        autoFlow="column"
+        columnGap="size-250"
+        rowGap="size-150"
+        columns={repeat(4, 'size-2000')}
+        rows={repeat(8, 'size-400')}
+      >
+        <label>Button style=&quot;outline&quot;</label>
+        <Button variant="primary" style="outline">
+          Primary
+        </Button>
+        <Button variant="secondary" style="outline">
+          Secondary
+        </Button>
+        <Button variant="accent" style="outline">
+          Accent
+        </Button>
+        <Button variant="negative" style="outline">
+          Negative
+        </Button>
+        <Button variant="primary" staticColor="black" style="outline">
+          Static Black
+        </Button>
+        <Button variant="primary" staticColor="white" style="outline">
+          Static White
+        </Button>
+        <Button variant="primary" isDisabled style="outline">
+          Disabled
+        </Button>
 
-      <label>Button style=&quot;fill&quot;</label>
-      <Button variant="primary" style="fill">
-        Primary
-      </Button>
-      <Button variant="secondary" style="fill">
-        Secondary
-      </Button>
-      <Button variant="accent" style="fill">
-        Accent
-      </Button>
-      <Button variant="negative" style="fill">
-        Negative
-      </Button>
-      <Button variant="primary" staticColor="black" style="fill">
-        Static Black
-      </Button>
-      <Button variant="primary" staticColor="white" style="fill">
-        Static White
-      </Button>
-      <Button variant="primary" isDisabled style="fill">
-        Disabled
-      </Button>
+        <label>Button style=&quot;fill&quot;</label>
+        <Button variant="primary" style="fill">
+          Primary
+        </Button>
+        <Button variant="secondary" style="fill">
+          Secondary
+        </Button>
+        <Button variant="accent" style="fill">
+          Accent
+        </Button>
+        <Button variant="negative" style="fill">
+          Negative
+        </Button>
+        <Button variant="primary" staticColor="black" style="fill">
+          Static Black
+        </Button>
+        <Button variant="primary" staticColor="white" style="fill">
+          Static White
+        </Button>
+        <Button variant="primary" isDisabled style="fill">
+          Disabled
+        </Button>
 
-      <label>Action Button</label>
-      <ActionButton>Normal</ActionButton>
-      <ActionButton gridRow="span 3" isQuiet>
-        Quiet
-      </ActionButton>
-      <ActionButton staticColor="black">Static Black</ActionButton>
-      <ActionButton staticColor="white">Static White</ActionButton>
-      <ActionButton isDisabled>Disabled</ActionButton>
+        <label>Action Button</label>
+        <ActionButton>Normal</ActionButton>
+        <ActionButton gridRow="span 3" isQuiet>
+          Quiet
+        </ActionButton>
+        <ActionButton staticColor="black">Static Black</ActionButton>
+        <ActionButton staticColor="white">Static White</ActionButton>
+        <ActionButton isDisabled>Disabled</ActionButton>
 
-      <label>Toggle Button</label>
-      <ToggleButton>Normal</ToggleButton>
-      <ToggleButton isQuiet>Quiet</ToggleButton>
-      <ToggleButton gridRow="span 2" isEmphasized>
-        Emphasized
-      </ToggleButton>
-      <ToggleButton staticColor="black">Static Black</ToggleButton>
-      <ToggleButton staticColor="white">Static White</ToggleButton>
-      <ToggleButton isDisabled>Disabled</ToggleButton>
-    </Grid>
+        <label>Toggle Button</label>
+        <ToggleButton>Normal</ToggleButton>
+        <ToggleButton isQuiet>Quiet</ToggleButton>
+        <ToggleButton gridRow="span 2" isEmphasized>
+          Emphasized
+        </ToggleButton>
+        <ToggleButton staticColor="black">Static Black</ToggleButton>
+        <ToggleButton staticColor="white">Static White</ToggleButton>
+        <ToggleButton isDisabled>Disabled</ToggleButton>
+      </Grid>
+    </>
   );
 }
 
@@ -188,7 +196,7 @@ function IllustratedMessageSample(): JSX.Element {
   return (
     <IllustratedMessage>
       <Icon size="XL">
-        <FontAwesomeIcon icon={dh.vsEmptyWindow} />
+        <FontAwesomeIcon icon={vsEmptyWindow} />
       </Icon>
       <Heading>Illustrated Message</Heading>
       <Content>This is the content of the message.</Content>

diff --git a/packages/components/scss/BaseStyleSheet.scss b/packages/components/scss/BaseStyleSheet.scss
@@ -3,6 +3,11 @@
 @import './custom.scss';
 @import 'bootstrap/scss/bootstrap';
 
+:root {
+  --dh-svg-inline-icon-size: 16px;
+  --scrollbar-color: 255, 255, 255;
+}
+
 //Various non-variable css overides
 //Overide default size from 16px to 14px. We need density.
 html {
@@ -14,7 +19,7 @@ html {
 // and look best at that size. Default icon size back to 16px
 // vertical alignment changed to best match icon set against 14px text
 .svg-inline--fa {
-  font-size: 16px;
+  font-size: var(--dh-svg-inline-icon-size);
   vertical-align: -3px;
 }
 
@@ -656,12 +661,6 @@ input[type='number']::-webkit-inner-spin-button {
   background: rgba(var(--scrollbar-color), 0.35);
 }
 
-/* stylelint-disable no-descending-specificity */
-:root {
-  --scrollbar-color: 255, 255, 255;
-}
-/* stylelint-enable no-descending-specificity */
-
 // used in inverted color sections, like light modals
 .theme-bg-light {
   --scrollbar-color: 0, 0, 0;

diff --git a/packages/components/src/__snapshots__/SpectrumUtils.test.ts.snap b/packages/components/src/__snapshots__/SpectrumUtils.test.ts.snap
@@ -5,7 +5,6 @@ exports[`themeDHDefault should merge Spectrum default with DH custom styles 1`]
   "dark": {
     "spectrum--darkest": "spectrum--darkest_256eeb",
     "theme-spectrum-alias": "theme-spectrum-alias",
-    "theme-spectrum-overrides": "theme-spectrum-overrides",
     "theme-spectrum-palette": "theme-spectrum-palette",
   },
   "global": {
@@ -23,7 +22,6 @@ exports[`themeDHDefault should merge Spectrum default with DH custom styles 1`]
   "light": {
     "spectrum--light": "spectrum--light_a40724",
     "theme-spectrum-alias": "theme-spectrum-alias",
-    "theme-spectrum-overrides": "theme-spectrum-overrides",
     "theme-spectrum-palette": "theme-spectrum-palette",
   },
   "medium": {

diff --git a/packages/components/src/theme/__snapshots__/ThemeProvider.test.tsx.snap b/packages/components/src/theme/__snapshots__/ThemeProvider.test.tsx.snap
@@ -9,7 +9,7 @@ exports[`ThemeProvider setSelectedThemeKey: [ [Object] ] should change selected
       ./theme-light-palette.css?raw
     </style>
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -39,7 +39,7 @@ exports[`ThemeProvider setSelectedThemeKey: [ [Object] ] should change selected
       data-theme-key="themeA"
     />
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -56,7 +56,7 @@ exports[`ThemeProvider setSelectedThemeKey: null should change selected theme: d
 <body>
   <div>
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -73,7 +73,7 @@ exports[`ThemeProvider setSelectedThemeKey: null should change selected theme: t
 <body>
   <div>
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -103,7 +103,7 @@ exports[`ThemeProvider should load themes based on preload data or default: [ [O
       data-theme-key="themeA"
     />
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -130,7 +130,7 @@ exports[`ThemeProvider should load themes based on preload data or default: [ [O
 ./theme-dark-components.css?raw
     </style>
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -147,7 +147,7 @@ exports[`ThemeProvider should load themes based on preload data or default: null
 <body>
   <div>
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"
@@ -164,7 +164,7 @@ exports[`ThemeProvider should load themes based on preload data or default: null
 <body>
   <div>
     <div
-      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias theme-spectrum-overrides spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
+      class="spectrum_b37d53 spectrum_2a241c spectrum--light_a40724 theme-spectrum-palette theme-spectrum-alias spectrum--large_c40598 spectrum_9e130c spectrum--medium_9e130c spectrum--large_9e130c spectrum--darkest_9e130c spectrum--dark_9e130c spectrum--light_9e130c spectrum--lightest_9e130c"
       dir="ltr"
       lang="en-US"
       style="isolation: isolate; color-scheme: light;"

diff --git a/packages/components/src/theme/theme-spectrum/index.ts b/packages/components/src/theme/theme-spectrum/index.ts
@@ -1,6 +1,8 @@
 import palette from './theme-spectrum-palette.module.css';
 import alias from './theme-spectrum-alias.module.css';
-import overrides from './theme-spectrum-overrides.module.css';
+
+// don't import as module, so that classes can be used directly
+import './theme-spectrum-overrides.css';
 
 /**
  * Spectrum theme variables are exported as a map of css class names. The keys
@@ -16,7 +18,6 @@ import overrides from './theme-spectrum-overrides.module.css';
 export const themeSpectrumClassesCommon = {
   ...palette,
   ...alias,
-  ...overrides,
 };
 
 export default themeSpectrumClassesCommon;
diff --git a/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css b/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.css
@@ -0,0 +1,13 @@
+[class^='spectrum'] {
+  /* --font-family-sans-serif is defined in Bootstrap's _root.scss. We want
+  Spectrum to use the same default font-family */
+  font-family: var(--font-family-sans-serif) !important;
+}
+
+.svg-inline--fa[class*='spectrum-Icon--sizeS'] {
+  /* 
+  Resize fontawesome icons used inside a spectrum icon wrapper to match
+  our icon size. Spectrum icons are 18px by default, but our icons are built on a 16px grid.
+  */
+  --spectrum-alias-workflow-icon-size: var(--dh-svg-inline-icon-size);
+}
diff --git a/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.module.css b/packages/components/src/theme/theme-spectrum/theme-spectrum-overrides.module.css
diff --git a/tests/styleguide.spec.ts-snapshots/spectrum-buttons-chromium-linux.png b/tests/styleguide.spec.ts-snapshots/spectrum-buttons-chromium-linux.png
diff --git a/tests/styleguide.spec.ts-snapshots/spectrum-buttons-firefox-linux.png b/tests/styleguide.spec.ts-snapshots/spectrum-buttons-firefox-linux.png
diff --git a/tests/styleguide.spec.ts-snapshots/spectrum-buttons-webkit-linux.png b/tests/styleguide.spec.ts-snapshots/spectrum-buttons-webkit-linux.png