styled-components.dropdown menu items #616

interim17 · 2024-12-04T15:09:58Z

Time estimate or Size

Medium
Advances #601

Problem

Dropdown menu elements can be fussy to style as the base html elements receive different default, focus and hover rules front ant

Solution

As part of migrating to ant5 I wanted to start using some CSS-in-JS and styled-components is in use in other AICS tools like timelapse-colorizer. Setting up styled-components during the ant migration makes sense to me, if its something we might like to use.

`DropdownMenuItem`

This file uses styled-components to create a uniformly styled element for dropdown menu items, regardless of whether the item is a button, an anchor tag, or a react router link.

It applies the common styles and then exposes three different React.FC exports for use in the menus.

token/theme reorganization

Preferable to use JS instead of CSS variables in the styled-components which led me to once again reorganize the colors and theming.

I think this version is better, the complete set of base colors from the Figma style guide is included, a couple base colors have been consolidated, I've reduced redundancy as much as possible, and the whole set of base, semantic, and component colors is available via the ThemeProvider

StyleProvider

Made a wrapper for the now accumulating styling providers so that App doesn't look cluttered.

New feature (non-breaking change which adds functionality)

…n items

…f themeColors

interim17 · 2024-12-04T15:11:57Z

src/components/CustomDropdown/DropdownMenuItems.tsx

+
+// Common styles
+const baseStyles = css`
+    font-family: ${(props) => props.theme.typography};


this is our only styled component for now, but we will probably want to add a GlobalStyles provider to get things like basic fonts into all styled components, and it could potentially replace some or all of the global style rules in style.css if we do that.

interim17 · 2024-12-04T15:12:54Z

src/components/CustomDropdown/DropdownMenuItems.tsx

+    width: 100%;
+    font-size: 14px;
+
+    &&& {


&& (or here &&&) This is a nice way in sass/styled-components to do a "precedence boost" without chasing down lots of antd class names, and without using !important which overlooks specificity entirely.

https://styled-components.com/docs/basics#adapting-based-on-props

interim17 · 2024-12-05T20:02:43Z

src/styles/theme/colors.ts

+        four: "#25222e", // fka three
+        five: "#2f2a3c", // fka four
+        six: "#3b3649", // fka charcoal gray, its purplish...


Can remove these comments, but in our *migration era* I think these are time savers when juggling old css vars and new color constants

migration era ✨

interim17 · 2024-12-05T20:05:09Z

src/styles/theme/colors.ts

+export const semanticColors = {
+    // bg colors
+    lightBg: baseColors.gray.one,
+    mediumBg: baseColors.dark.six,
+    grayBg: baseColors.gray.four,
+    darkBg: baseColors.dark.four,
+    lightPurpleBg: baseColors.purple.one,
+    // text colors
+    lightBgText: baseColors.dark.one,
+    lightBgActiveText: baseColors.dark.three,
+    darkBgText: baseColors.gray.nine,
+    whiteText: baseColors.white.pure,
+    // accent colors
+    primaryPurple: baseColors.purple.six,
+    activePurple: baseColors.purple.five,
+    primaryBlue: baseColors.blue,
+    primaryBorder: baseColors.gray.seven,
+    sliderColor: baseColors.gray.six,
+    buttonTextGray: baseColors.gray.five,
+    tagBg: baseColors.purple.nine,
+    tagText: baseColors.gray.nine,
+};


I think these can/will continue to evolve a bit as we dial in all styling, but file now has a basic way of exporting base colors, these semantic colors, and then component specific assignments. Spreading all three into the themeColors export makes any layer the color hierarchy (base, semantic, component) available via styled components theme props.

Would it make sense to not export base colors directly? That way you push development towards assigning specific semantic colors or component colors instead of relying on the hardcoded color directly.

So, if I'm understanding you I think I do at the bottom of the file?

export const themeColors = { ...baseColors, ...semanticColors, ...componentColors, };

I'm trying to balance both exporting all the most specific instances of the colors for use wherever they end up being needed, and the feedback I've received on previous PRs to consolidate anything that gets used repeatedly in similar contexts into these semantic variables.

Yes, that's where I meant!

My question was whether baseColors should be included, since it allows components to assign specific colors instead of requiring them to go through an alias. This creates a dependency on those colors specifically, and means that changing any of them would potentially have a lot of unintended consequences in the future.

ShrimpCryptid

Left a bunch of comments!

ShrimpCryptid · 2024-12-10T17:50:51Z

src/components/CustomDropdown/DropdownMenuItems.tsx

+
+// Common styles
+const baseStyles = css`
+    font-family: ${(props) => props.theme.typography};


Does TS pick up that props is of a specific type, or is it any? Are you able to specify the type of theme?

According to intellisense the type isn't any it's ExecutionContext & object which I'm not entirely sure about, I can type theme as DefaultTheme from styled-components in the file where I export it, but that type is already picked up here in this file.

I'm not getting a typescript error, do you type props more explicitly in your implementation of styled-components in TFE?

I haven't used the props.theme before in StyledComponents, I usually am getting my theme variables out of a context. Just curious if Intellisense/TS picked up the Defaulttheme type!

It did, and the ThemeProvider is a essentially a context

src/components/CustomDropdown/DropdownMenuItems.tsx

ShrimpCryptid · 2024-12-10T18:01:49Z

src/components/CustomDropdown/DropdownMenuItems.tsx

+const StyledDropdownButton = styled.button`
+    ${baseStyles}
+    ${contentStyles}
+`;
+
+const StyledRouterLink = styled(Link)`
+    ${baseStyles}
+    ${contentStyles}
+`;
+
+const StyledExternalLink = styled.a`
+    ${baseStyles}
+    ${contentStyles}
+`;


✨✨✨✨✨✨

src/components/HelpMenu/index.tsx

ShrimpCryptid · 2024-12-10T18:12:27Z

src/styles/theme/colors.ts

+        four: "#25222e", // fka three
+        five: "#2f2a3c", // fka four
+        six: "#3b3649", // fka charcoal gray, its purplish...


migration era ✨

ShrimpCryptid · 2024-12-10T18:14:59Z

src/styles/theme/colors.ts

+export const semanticColors = {
+    // bg colors
+    lightBg: baseColors.gray.one,
+    mediumBg: baseColors.dark.six,
+    grayBg: baseColors.gray.four,
+    darkBg: baseColors.dark.four,
+    lightPurpleBg: baseColors.purple.one,
+    // text colors
+    lightBgText: baseColors.dark.one,
+    lightBgActiveText: baseColors.dark.three,
+    darkBgText: baseColors.gray.nine,
+    whiteText: baseColors.white.pure,
+    // accent colors
+    primaryPurple: baseColors.purple.six,
+    activePurple: baseColors.purple.five,
+    primaryBlue: baseColors.blue,
+    primaryBorder: baseColors.gray.seven,
+    sliderColor: baseColors.gray.six,
+    buttonTextGray: baseColors.gray.five,
+    tagBg: baseColors.purple.nine,
+    tagText: baseColors.gray.nine,
+};


Would it make sense to not export base colors directly? That way you push development towards assigning specific semantic colors or component colors instead of relying on the hardcoded color directly.

…onents theme

ShrimpCryptid

LGTM!

interim17 added 5 commits December 3, 2024 10:24

use styled-component to streamline appearance of dropdown menu items

8f3e2a7

prevent ant from overridding text color during hover/focus on dropdow…

bda00ac

…n items

trim unneedd props from baseStyles on dropdown items

2728d89

restructure colors and theming to provide for styled-components use o…

5e99df2

…f themeColors

remove unused nav button drop down item variety

0eff0aa

interim17 commented Dec 4, 2024

View reviewed changes

don't open drop downs by default

7b9cfd5

interim17 mentioned this pull request Dec 4, 2024

Feature/config provider #614

Open

interim17 added 3 commits December 5, 2024 11:24

remove unused imports

bfba41d

delete theme config

14b493f

make selector more specific and remove !important tag

c7b0ae1

interim17 commented Dec 5, 2024

View reviewed changes

interim17 marked this pull request as ready for review December 5, 2024 20:05

interim17 requested a review from a team as a code owner December 5, 2024 20:05

interim17 requested review from meganrm and ShrimpCryptid and removed request for a team December 5, 2024 20:05

ShrimpCryptid reviewed Dec 10, 2024

View reviewed changes

change focus to focus-visible in selectors and typing for styled-comp…

60bbf94

…onents theme

interim17 requested a review from ShrimpCryptid December 15, 2024 21:06

only export semantic and component colors from theme

8a8cc43

ShrimpCryptid approved these changes Dec 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

styled-components.dropdown menu items #616

styled-components.dropdown menu items #616

interim17 commented Dec 4, 2024 •

edited

Loading

interim17 Dec 4, 2024 •

edited

Loading

interim17 Dec 4, 2024 •

edited

Loading

interim17 Dec 5, 2024 •

edited

Loading

ShrimpCryptid Dec 10, 2024

interim17 Dec 5, 2024

ShrimpCryptid Dec 10, 2024

interim17 Dec 12, 2024

ShrimpCryptid Dec 13, 2024

ShrimpCryptid left a comment

ShrimpCryptid Dec 10, 2024

interim17 Dec 12, 2024

ShrimpCryptid Dec 13, 2024

interim17 Dec 17, 2024

ShrimpCryptid Dec 10, 2024

ShrimpCryptid Dec 10, 2024

ShrimpCryptid Dec 10, 2024

ShrimpCryptid left a comment

styled-components.dropdown menu items #616

Are you sure you want to change the base?

styled-components.dropdown menu items #616

Conversation

interim17 commented Dec 4, 2024 • edited Loading

Time estimate or Size

Problem

Solution

DropdownMenuItem

token/theme reorganization

StyleProvider

interim17 Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

interim17 Dec 4, 2024 • edited Loading

Choose a reason for hiding this comment

interim17 Dec 5, 2024 • 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

ShrimpCryptid left a 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

ShrimpCryptid left a comment

Choose a reason for hiding this comment

interim17 commented Dec 4, 2024 •

edited

Loading

`DropdownMenuItem`

interim17 Dec 4, 2024 •

edited

Loading

interim17 Dec 4, 2024 •

edited

Loading

interim17 Dec 5, 2024 •

edited

Loading