Merge pull request #331 from storybookjs/select-focus-color

Improve Select styling and fix Safari bug
storybookjs · Jan 22, 2022 · 1f767f7 · 1f767f7
2 parents 33fac59 + 14b6052
commit 1f767f7
Show file tree

Hide file tree

Showing 4 changed files with 805 additions and 816 deletions.
diff --git a/package.json b/package.json
@@ -74,14 +74,14 @@
     "@babel/preset-env": "^7.15.6",
     "@babel/preset-react": "^7.10.1",
     "@babel/preset-typescript": "^7.10.1",
-    "@storybook/addon-a11y": "^6.3.8",
-    "@storybook/addon-actions": "^6.3.8",
-    "@storybook/addon-docs": "^6.3.8",
-    "@storybook/addon-essentials": "^6.3.8",
-    "@storybook/addon-storysource": "^6.3.8",
+    "@storybook/addon-a11y": "^6.4.13",
+    "@storybook/addon-actions": "^6.4.13",
+    "@storybook/addon-docs": "^6.4.13",
+    "@storybook/addon-essentials": "^6.4.13",
+    "@storybook/addon-storysource": "^6.4.13",
     "@storybook/eslint-config-storybook": "^2.4.0",
     "@storybook/linter-config": "^2.5.0",
-    "@storybook/react": "^6.3.8",
+    "@storybook/react": "^6.4.13",
     "@svgr/cli": "^6.1.1",
     "@types/fs-extra": "^9.0.1",
     "auto": "^9.50.1",

diff --git a/src/components/Select.stories.tsx b/src/components/Select.stories.tsx
@@ -1,15 +1,17 @@
 import React from 'react';
 import { action } from '@storybook/addon-actions';
+import { styled } from '@storybook/theming';
 
-import { Select } from './Select';
+import { Select as UnstyledSelect } from './Select';
 
 const onChange = action('change');
 
 export default {
   title: 'forms/Select',
-  component: Select,
+  component: UnstyledSelect,
 };
 
+
 export const Template = (args) => <Select {...args} />;
 Template.args = {
   value: 'value1',
@@ -21,40 +23,23 @@ Template.args = {
 };
 Template.story = { name: 'Playground' };
 
-export const All = () => (
-  <form>
-    <Select
-      id="Primary"
-      value="value1"
-      options={[
-        { label: 'Default', value: 'value1' },
-        { label: 'Dog', value: 'value2' },
-        { label: 'Mouse', value: 'value3' },
-      ]}
-      onChange={onChange}
-      label="Animal"
-      hideLabel
-    />
-    <Select
-      id="Tertiary"
-      value="value1"
-      options={[
-        { label: 'Tertiary', value: 'value1' },
-        { label: 'Dog', value: 'value2' },
-        { label: 'Mouse', value: 'value3' },
-      ]}
-      appearance="tertiary"
-      onChange={onChange}
-      label="Animal"
-      hideLabel
-    />
-  </form>
-);
 
-export const Default = () => (
-  <form>
+const Form = styled.form`
+  padding: 3em 12em;
+`;
+
+const DarkForm = styled(Form)`
+  background: #eeeeee;
+`;
+
+const Select = styled(UnstyledSelect)`
+  padding-top: 1em;
+`;
+
+const All = ({ appearance }) => (
+  <>
     <Select
-      id="Primary"
+      id="Default"
       label="Animal"
       hideLabel
       value="value1"
@@ -63,109 +48,99 @@ export const Default = () => (
         { label: 'Dog', value: 'value2' },
       ]}
       onChange={onChange}
+      appearance={appearance}
+
     />
     <Select
-      id="Primary-disabled"
+      id="disabled"
       label="Animal"
       hideLabel
       value="value1"
       options={[
-        { label: 'Default', value: 'value1' },
+        { label: 'Disabled', value: 'value1' },
         { label: 'Dog', value: 'value2' },
       ]}
       disabled
       onChange={onChange}
+      appearance={appearance}
     />
     <Select
-      id="Primary-with-iconn"
+      id="with-icon"
       label="Animal"
       hideLabel
       value="value1"
       options={[
-        { label: 'Default', value: 'value1' },
+        { label: 'With icon', value: 'value1' },
         { label: 'Dog', value: 'value2' },
       ]}
       icon="chromatic"
       onChange={onChange}
+      appearance={appearance}
     />
     <Select
-      id="Primary-in-progress"
+      id="in-progress-with-icon"
       label="Animal"
       hideLabel
       value="value1"
       options={[
-        { label: 'Default', value: 'value1' },
+        { label: 'In progress with icon', value: 'value1' },
         { label: 'Dog', value: 'value2' },
       ]}
       icon="chromatic"
       onChange={onChange}
+      appearance={appearance}
       inProgress
     />
     <Select
-      id="Primary-with-error"
+      id="with-error"
       label="Animal"
       hideLabel
       value="value1"
       options={[
-        { label: 'Default', value: 'value1' },
+        { label: 'With error', value: 'value1' },
         { label: 'Dog', value: 'value2' },
       ]}
       error="There's a snake in my boots"
       onChange={onChange}
+      appearance={appearance}
     />
     <Select
-      id="Primary-with-icon-and-error"
+      id="with-icon-and-error"
       label="Animal"
       hideLabel
       value="value1"
       options={[
-        { label: 'Default', value: 'value1' },
+        { label: 'With icon and error', value: 'value1' },
         { label: 'Dog', value: 'value2' },
       ]}
       icon="chromatic"
       error="There's a snake in my boots"
       onChange={onChange}
+      appearance={appearance}
     />
     <Select
-      id="Primary-with-label"
+      id="with-label"
       value="value1"
       options={[
-        { label: 'Default', value: 'value1' },
+        { label: 'With label', value: 'value1' },
         { label: 'Dog', value: 'value2' },
       ]}
       icon="chromatic"
       label="Label"
       onChange={onChange}
+      appearance={appearance}
     />
-  </form>
+  </>
+);
+
+export const Default = () => (
+  <DarkForm>
+    <All appearance="default" />
+  </DarkForm>
 );
 
 export const Tertiary = () => (
-  <form>
-    <Select
-      id="Tertiary"
-      label="Animal"
-      hideLabel
-      value="value1"
-      options={[
-        { label: 'Default', value: 'value1' },
-        { label: 'Dog', value: 'value2' },
-      ]}
-      appearance="tertiary"
-      onChange={onChange}
-    />
-    <Select
-      id="Tertiary-disabled"
-      label="Animal"
-      hideLabel
-      value="value1"
-      options={[
-        { label: 'Default', value: 'value1' },
-        { label: 'Dog', value: 'value2' },
-      ]}
-      disabled
-      appearance="tertiary"
-      onChange={onChange}
-    />
-  </form>
+  <Form>
+    <All appearance="tertiary" />
+  </Form>
 );
diff --git a/src/components/Select.tsx b/src/components/Select.tsx
@@ -48,6 +48,7 @@ const Selector = styled.select<SelectProps>`
   position: relative;
   outline: none;
   width: 100%;
+  margin: 0;
 
   ${(props) =>
     props.disabled &&
@@ -86,7 +87,7 @@ const SelectError = styled.div`
   font-family: ${typography.type.primary};
 
   color: ${color.negative};
-  line-height: 38px;
+  line-height: 40px;
   padding-right: 2.75em;
 `;
 
@@ -95,6 +96,7 @@ interface WrapperProps {
   appearance: 'default' | 'tertiary';
   hasIcon: boolean;
   error: any;
+  inProgress: boolean;
 }
 
 const SelectWrapper = styled.span<WrapperProps>`
@@ -146,9 +148,10 @@ const SelectWrapper = styled.span<WrapperProps>`
 
   ${Selector} {
     box-shadow: ${color.border} 0 0 0 1px inset;
+    border-radius: 4px;
   }
   ${Selector}:focus {
-    box-shadow: ${color.primary} 0 0 0 1px inset;
+    box-shadow: ${color.secondary} 0 0 0 1px inset;
   }
 
   ${(props) =>
@@ -157,9 +160,6 @@ const SelectWrapper = styled.span<WrapperProps>`
       opacity: 0.5;
     `}
 
-  &:before {
-    background-color: rgba(255, 255, 255, 0.9);
-  }
   ${Selector} {
     background-color: ${color.lightest};
     color: ${color.darkest};
@@ -183,21 +183,34 @@ const SelectWrapper = styled.span<WrapperProps>`
       }
       ${Arrow} {
         right: 0;
+        top: 12px;
+      }
+      ${SelectSpinner} {
+        right: 0;
+      }
+      ${SelectIcon} {
+        left: 0;
+        top: 12px;
+      }
+      ${SelectError} {
+        position: relative;
+        line-height: 20px;
+        margin-top: 8px;
       }
     `}
 
   ${(props) =>
     props.hasIcon &&
     `
       ${Selector} {
-        padding-left: 2.5em;
+        padding-left: ${props.appearance === 'tertiary' ? '20px' : '40px'};
       }
 
       ${Selector} + ${SelectIcon} {
         transition: all 150ms ease-out;
         position: absolute;
-        top: 50%;
-        left: 0.8em;
+        top: ${props.appearance === 'tertiary' ? '12px' : '50%'};
+        left: ${props.appearance === 'tertiary' ? 0 : '0.8em'};
         height: 1em;
         width: 1em;
         margin-top: -0.5em;
@@ -290,6 +303,7 @@ export const Select: FunctionComponent<Props & ComponentProps<typeof Selector>>
         data-error={error}
         error={error}
         disabled={disabled}
+        inProgress={inProgress}
       >
         {!inProgress && <Arrow />}
         <Selector