[ui] Themes #18442

hellendag · 2023-12-01T18:26:41Z

Summary & Motivation

Introduce a system for theming the Dagster UI. All direct references to color values are replaced by a set of functions that match semantic color names to theme-based palette values.

The theme value is exposed via a User Setting.

Discussion here: #18439

How I Tested These Changes

TS, lint, jest. Find all existing references to LegacyColors, hex values, rgb/rgba values and replace them with color functions.

Navigate throughout the app in legacy, light, and dark modes. Verify that colors look appropriate.

## Summary & Motivation Add a legacy theme to keep the colors mostly unchanged while working on the new palettes. ## How I Tested These Changes

## Summary & Motivation Adds a bunch of small style tweaks to the dark and light themes ## How I Tested These Changes Booted up the UI and tried it

[INTERNAL_BRANCH=dish/cloud-palettes]

## Summary & Motivation More theme style tweaks ## How I Tested These Changes Booted up the UI --------- Co-authored-by: Isaac Hellendag <[email protected]> Co-authored-by: Isaac Hellendag <[email protected]>

github-actions · 2023-12-01T18:30:03Z

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-8gv6y3rie-elementl.vercel.app
https://dish-palettes.components-storybook.dagster-docs.io

Built with commit fd59242.
This pull request is being automatically deployed with vercel-action

github-actions · 2023-12-01T18:33:09Z

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-3h4x9d4bj-elementl.vercel.app
https://dish-palettes.core-storybook.dagster-docs.io

Built with commit fd59242.
This pull request is being automatically deployed with vercel-action

bengotow

This looks awesome. The approach seems solid and I'm a big fan of the semantic naming of the color values. Props for going through and re-classifying everything, this must have taken a while! 💯

bengotow · 2023-12-02T00:00:01Z

js_modules/dagster-ui/packages/app-oss/src/NUX/CommunityNux.tsx

@@ -101,7 +103,7 @@ const Form = ({dismiss, submit}: FormProps) => {
      >
        <Box flex={{direction: 'column', gap: 8, alignItems: 'start', justifyContent: 'start'}}>
          <Heading>Join the Dagster community</Heading>
-          <Body style={{color: Colors.Gray700, marginBottom: '4px'}}>


Unrelated, but was moving to base exports from ui-components explicit? I guess this could have stayed Colors.textLight(), curious if that'd be better / worse (though arguably not better enough to be worth changing now!)

Yeah, I guess we could have the functions namespaced under Colors. If you guys feel strongly about doing that instead, we could do a find/replace for it.

bengotow · 2023-12-02T00:00:45Z

js_modules/dagster-ui/packages/app-oss/src/NUX/CommunityNux.tsx

          style={{width: '100%'}}
        />
        {emailChanged && blurred && !validEmail ? (
-          <div style={{paddingBottom: '12px', color: Colors.Red500, fontSize: '12px'}}>


Noticed that this "Red500" and the above "Red500" were changed differently based on the context of use which is awesome but... must have taken a while. Thanks for doing all this!

bengotow · 2023-12-02T00:04:26Z

js_modules/dagster-ui/packages/ui-components/src/components/Checkbox.tsx

@@ -35,31 +45,34 @@ const StarIcon = ({checked, indeterminate, fillColor, disabled}: IconProps) => (
    <path
      className="interaction-focus-outline"
      d="M8.99983 14.27L13.1498 16.78C13.9098 17.24 14.8398 16.56 14.6398 15.7L13.5398 10.98L17.2098 7.80001C17.8798 7.22001 17.5198 6.12001 16.6398 6.05001L11.8098 5.64001L9.91983 1.18001C9.57983 0.37001 8.41983 0.37001 8.07983 1.18001L6.18983 5.63001L1.35983 6.04001C0.479829 6.11001 0.119828 7.21001 0.789828 7.79001L4.45983 10.97L3.35983 15.69C3.15983 16.55 4.08983 17.23 4.84983 16.77L8.99983 14.27Z"
-      fill={Colors.White}
+      fill={disabled ? colorBackgroundGray() : colorBackgroundDefault()}


I really like the strategy you've taken here (color functions) as opposed to using styled-components Theme support or @media(prefer-color-scheme: dark). I've seen other apps that try to sync to the system theme via CSS, but then /every single color reference/ needs to be in CSS (or tied to fancy media-watching JS) because the value can change without a page reload.

I like that this solution still lets us interpolate the color at render time and slap it wherever we need to. Slight bummer that at 5:00pm it won't auto-shift to dark mode without a page reload, but the engineering cost for that one page load is HIGH and not worth it.

I don't think I've seen the 5pm shift before! I'm just always in dark mode. We could come back to this if it seems important to users.

bengotow · 2023-12-02T00:04:57Z

js_modules/dagster-ui/packages/ui-components/src/palettes/LightPalette.tsx

+  [ColorName.DataVizVioletAlt]: DataVizColors.Violet300,
+  [ColorName.DataVizYellow]: DataVizColors.Yellow200,
+  [ColorName.DataVizYellowAlt]: DataVizColors.Yellow300,
+};


I like the DataViz family names :)

bengotow · 2023-12-02T00:05:25Z

js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNode.tsx

+    border-bottom-left-radius: 8px;
+    border-bottom-right-radius: 8px;


Unrelated but seems fine!

bengotow · 2023-12-02T00:06:13Z

js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNode.tsx

  position: relative;
+  transition: all 150ms linear;


Ooh does this animate the border change on click?

bengotow · 2023-12-02T00:06:44Z

js_modules/dagster-ui/packages/ui-core/src/asset-graph/AssetNode.tsx

    box-shadow: rgba(0, 0, 0, 0.12) 0px 2px 12px 0px;
+    scale: 1.03;


This one is a bit interesting (and new i think?), might be worth a comment about what this does

I did added this one. It adds a slight scale effect on hover. We have the same effect on the new group nodes as well. happy to revisit this if y'all find it distracting but the primary motivation for adding it was that we previously relied on a box shadow for the node hover style, and this doesn't show up well in dark mode. The scale transition works well in both light and dark mode

## Summary & Motivation Introduce a system for theming the Dagster UI. All direct references to color values are replaced by a set of functions that match semantic color names to theme-based palette values. The theme value is exposed via a User Setting. Discussion here: dagster-io#18439 ## How I Tested These Changes TS, lint, jest. Find all existing references to `LegacyColors`, hex values, rgb/rgba values and replace them with color functions. Navigate throughout the app in legacy, light, and dark modes. Verify that colors look appropriate. --------- Co-authored-by: Josh Braun <[email protected]>

hellendag and others added 23 commits December 1, 2023 12:19

[ui] WIP: Themeing and updated color palette

db88b95

Add legacy palette (#18230)

176111f

## Summary & Motivation Add a legacy theme to keep the colors mostly unchanged while working on the new palettes. ## How I Tested These Changes

Some tweaks from storybook investigation, refactor button colors

2682ea7

Tweaks to dialog, error trace, logs

61779fb

Background wash, type schema

f9c7bb4

Josh theme tweaks (#18284)

6969df1

## Summary & Motivation Adds a bunch of small style tweaks to the dark and light themes ## How I Tested These Changes Booted up the UI and tried it

Followups to Josh's tweaks

a0f25ff

Lint, fixes for bugs found by Josh

13ff7d9

Expose CoreColors for now

3fe869a

More fixes

68a4131

CodeMirror styles

a9060ef

Restore LegacyColors for now

1defe36

[INTERNAL_BRANCH=dish/cloud-palettes]

Tokenizing field

ee0e4a0

Stuff on Launchpad

4f5a227

Colors -> LegacyColors

9e7fbb7

Resolve conflicts etc

c6d1bd5

Dataviz color palette

9ad49ca

Try to fix storybook build

ed4a209

jb more theme updates (#18396)

a7300b4

## Summary & Motivation More theme style tweaks ## How I Tested These Changes Booted up the UI --------- Co-authored-by: Isaac Hellendag <[email protected]> Co-authored-by: Isaac Hellendag <[email protected]>

Tab hover state

569511d

Tooltip style

6919970

More adventures

8336fca

Update user settings, focus rings

fd59242

hellendag requested review from bengotow, salazarm, braunjj and leahpadgett December 1, 2023 18:27

bengotow approved these changes Dec 2, 2023

View reviewed changes

hellendag merged commit e5bfb14 into master Dec 3, 2023
2 checks passed

hellendag deleted the dish/palettes branch December 3, 2023 16:10

hellendag mentioned this pull request Dec 4, 2023

Dark mode for Dagit #1890

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ui] Themes #18442

[ui] Themes #18442

hellendag commented Dec 1, 2023

github-actions bot commented Dec 1, 2023

github-actions bot commented Dec 1, 2023

bengotow left a comment

bengotow Dec 2, 2023 •

edited

Loading

hellendag Dec 3, 2023

bengotow Dec 2, 2023

bengotow Dec 2, 2023

hellendag Dec 3, 2023

bengotow Dec 2, 2023

bengotow Dec 2, 2023

bengotow Dec 2, 2023

bengotow Dec 2, 2023

braunjj Dec 2, 2023

		border-bottom-left-radius: 8px;
		border-bottom-right-radius: 8px;

		box-shadow: rgba(0, 0, 0, 0.12) 0px 2px 12px 0px;
		scale: 1.03;

[ui] Themes #18442

[ui] Themes #18442

Conversation

hellendag commented Dec 1, 2023

Summary & Motivation

How I Tested These Changes

github-actions bot commented Dec 1, 2023

github-actions bot commented Dec 1, 2023

bengotow left a comment

Choose a reason for hiding this comment

bengotow Dec 2, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bengotow Dec 2, 2023 •

edited

Loading