feat(forms): new light/dark mode colors #1838

jzempel · 2024-06-13T21:24:01Z

Description

Completed recolor work for Input, Checkbox, Radio, Toggle, Range, Select, FileUpload, File, FileList, FauxInput, MediaInput, and Tiles. Components inheriting this work include Textarea & InputGroup.

Detail

Note that Hint, Label, and Message were completed with #1816

Checklist

👌 design updates will be Garden Designer approved (add the designer as a reviewer)
🌐 demo is up-to-date (npm start)
⬅️ renders as expected with reversed (RTL) direction
🤘 renders as expected with Bedrock CSS (?bedrock)
💂‍♂️ includes new unit tests. Maintain existing coverage (always >= 96%)
♿ tested for WCAG 2.1 AA accessibility compliance
📝 tested in Chrome, Firefox, Safari, and Edge

jzempel · 2024-06-13T21:25:26Z

packages/forms/demo/fileUpload.stories.mdx

      ...commonArgs
    }}
    argTypes={{
+      tabIndex: { control: 'number' },


Needed (and has parity with File) to observe focus state.

jzempel · 2024-06-13T21:28:38Z

packages/theming/src/utils/getColor.ts

-      if (value.transparency !== undefined) {
-        _transparency = value.transparency;
-      }
+      _transparency = _transparency === undefined ? value.transparency : _transparency;


If transparency exists along with a variable parameter, the transparency should override the variable's rgba alpha value. This allows us to one-off tweak

getColor({ theme, variable: 'background.disabled', transparency: theme.opacity[300] });

as seen with StyledRadioInput.

jzempel · 2024-06-13T21:32:09Z

utils/test/jest.setup.js

+import { TextEncoder } from 'node:util';
+
+global.TextEncoder = TextEncoder;


Needed due to the addition of renderToString in StyledTextInput.

Looks like this matches browser behavior. 😰 I always get nervous about using node utils in tests when the component will leverage browser APIs in practice.

steue

a pass over everything sans Tiles and most looks good to me! 🔥 work! just one thing i noticed that has some implications on our Buttons package.

while reviewing InputGroup, it appears that the hover state of the neutral button is one shade too dark/light (blue-800/500) instead of using the expected blue-700/600 or border/primaryEmphasis.

the initial design intent for the neutral button was to visually align with inputs, including on hover treatment (but not active).

ive included a comparison diagram below. i’ve provided a visual of the intention below. but plz let me know if i might have missed something and this is intentional. sorry we didnt catch this earlier, as i know this affects a package we’ve already marked as done

p.s. since the changes would come from buttons, and the rest of the forms look good to me, i'll approve! ty again!

lucijanblagonic · 2024-06-14T09:16:20Z

File

Screenshots from deployment

Icon should be fg/error or fg/success depending on the error/success state.

We discussed this in Figma already, maybe I configured something wrong 🤔

jzempel · 2024-06-14T15:25:50Z

packages/forms/src/styled/select/StyledSelectWrapper.ts

+  const height = `${theme.space.base * (isCompact ? 8 : 10)}px`;
+
+  return css`
+    max-height: ${height};


Fixes a 2px height bump difference between Select and Input due to wrapper border.

Can we include an annotation? 👀

Yeah. In general, I'm really looking to getting rid of all this faux/wrapper/media/input differentiation in v9 with a new component.next that rolls everything into one.

jzempel · 2024-06-14T15:27:24Z

packages/forms/src/styled/tiles/StyledTile.ts

-interface IStyledTileProps {
-  isDisabled?: boolean;
-  isFocused?: boolean;
-  isSelected?: boolean;
-}


State info shouldn't be used when CSS can solve using :has()

jzempel · 2024-06-14T15:30:51Z

packages/forms/src/styled/tiles/StyledTileInput.ts

+  top: 0;
+  left: 0;
+  opacity: 0;
+  z-index: 1;
+  margin: 0;
+  cursor: ${props => (props.disabled ? 'default' : 'pointer')};
+  width: 100%;
+  height: 100%;


Now the visibly hidden input is truly interactive – similar to the approach used by ColorSwatch. This will allow us to revisit Tiles and add a isCheckboxGroup feature similar to ColorSwatch so that Tiles isn't constrained to behave as a radio-only group.

lucijanblagonic

Reviewed Tile (others were reviewed on Friday). Looks good. 🙌

geotrev

Couple small comments, but nothing blocking. Thanks for pushing the Select style fixes 🚀

geotrev · 2024-06-20T15:37:44Z

packages/forms/src/styled/range/StyledRangeInput.ts

+  });
+  const trackDisabledLowerBackgroundColor = hasLowerTrack
+    ? getColor({ theme, variable: 'border.emphasis' })
+    : '';


There's quite a few ternary's in this file (from the original implementation). Curious if we can get away with simplifying:

const trackDisabledLowerBackgroundColor = hasLowerTrack && getColor({ ... });

Yeah, I was thinking the same but resisted updates due to the focus of this PR. Let's try to revisit down the road.

geotrev · 2024-06-20T15:40:42Z

packages/forms/src/styled/select/StyledSelectWrapper.ts

+  const height = `${theme.space.base * (isCompact ? 8 : 10)}px`;
+
+  return css`
+    max-height: ${height};


Can we include an annotation? 👀

geotrev · 2024-06-20T15:59:11Z

packages/forms/src/styled/tiles/StyledTileIcon.ts

+      color: ${activeColor};
+    }
+
+    ${StyledTile}:has(:checked) && {


✨ new css features ftw

geotrev · 2024-06-20T16:14:33Z

utils/test/jest.setup.js

+import { TextEncoder } from 'node:util';
+
+global.TextEncoder = TextEncoder;


Looks like this matches browser behavior. 😰 I always get nervous about using node utils in tests when the component will leverage browser APIs in practice.

ze-flo

Nice work! 💯

ze-flo · 2024-06-20T17:37:50Z

packages/forms/src/styled/text/StyledTextInput.tsx

+  const readOnlyBackgroundColor = disabledBackgroundColor;
+  const calendarPickerColor = getColor({ theme, variable: 'foreground.subtle' });
+  const calendarPickerIcon = renderToString(<ChevronIcon color={calendarPickerColor} />);
+  const calendarPickerBackgroundImage = `url("data:image/svg+xml,${encodeURIComponent(calendarPickerIcon)}")`;


jzempel added 7 commits June 7, 2024 14:43

Recolor radio-based components

4d480a3

Merge branch 'next' into jzempel/recolor-forms

7de5ef8

Recolor text inputs

675ade9

Recolor select & range

f27d218

Recolor file components

d5b6f70

Update date input styling

5ab68c9

Format

01895e4

jzempel added the PR: New Feature 🚀 label Jun 13, 2024

jzempel requested review from lucijanblagonic, sieeeeeenna and steue June 13, 2024 21:24

jzempel requested a review from a team as a code owner June 13, 2024 21:24

jzempel marked this pull request as draft June 13, 2024 21:24

jzempel commented Jun 13, 2024

View reviewed changes

zendesk-garden deployed to staging June 13, 2024 21:32 View deployment

steue approved these changes Jun 14, 2024

View reviewed changes

jzempel added 3 commits June 14, 2024 10:17

Recolor tiles component

98535a1

Correct neutral button states for InputGroup parity

086598f

Fix File validation icon

ae5b924

zendesk-garden deployed to staging June 14, 2024 14:59 View deployment

Fix Select height

a1ce5e0

jzempel marked this pull request as ready for review June 14, 2024 15:16

zendesk-garden deployed to staging June 14, 2024 15:18 View deployment

jzempel commented Jun 14, 2024

View reviewed changes

lucijanblagonic approved these changes Jun 17, 2024

View reviewed changes

Fix disabled state colors for Select component

376c7ac

zendesk-garden deployed to staging June 20, 2024 16:25 View deployment

geotrev approved these changes Jun 20, 2024

View reviewed changes

Add CSS annotation

7a8044f

ze-flo approved these changes Jun 20, 2024

View reviewed changes

zendesk-garden deployed to staging June 20, 2024 17:45 View deployment

jzempel merged commit 4b79afd into next Jun 20, 2024

jzempel deleted the jzempel/recolor-forms branch June 20, 2024 17:49

		import { TextEncoder } from 'node:util';

		global.TextEncoder = TextEncoder;

feat(forms): new light/dark mode colors #1838

feat(forms): new light/dark mode colors #1838

Uh oh!

Conversation

jzempel commented Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Detail

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jzempel Jun 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

steue left a comment

Choose a reason for hiding this comment

Uh oh!

lucijanblagonic commented Jun 14, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lucijanblagonic left a comment

Choose a reason for hiding this comment

Uh oh!

geotrev left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ze-flo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

7 participants

jzempel commented Jun 13, 2024 •

edited

Loading

jzempel Jun 13, 2024 •

edited

Loading

lucijanblagonic commented Jun 14, 2024 •

edited

Loading