Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

style(ui): Update Select component styles #3944

Merged
merged 2 commits into from
Mar 24, 2025
Merged

Conversation

connieelee
Copy link
Contributor

@connieelee connieelee commented Mar 24, 2025

Description

Pre-req for https://wandb.atlassian.net/browse/WB-23683

We're building a new UI that displays a multi-select field next to a plain text field: https://www.figma.com/design/8v53a9ldK9DcVUkILt4CIn/Scratchpad?node-id=3102-175949&m=dev

But there are some styling discrepancies between our TextField and Select comps that are especially prominent when the two are displayed right next to each other.

Screenshot 2025-03-24 at 1 47 31 PM
  • height: multi-select is taller than TextField (40px vs 32px)
  • placeholder text: multi-select is smaller than TextField (14px vs 16px)
  • border-radius: multi-select is rounder than TextField (4px vs 2px)
  • "border": multi-select uses box-shadow to simulate border while TextField uses outline. this results in the borders to behave ever so slightly differently:
Screen.Recording.2025-03-24.at.2.00.31.PM.mov

There are also some styling issues with multi-select that are totally unrelated to TextField:

Screenshot 2025-03-24 at 1 52 19 PM
  • dropdown indicator (up/down chevron) should be hidden (see designs)
  • clear indicator should use our "close" icon
  • selected items should should have 2px gaps

This PR addresses all of the issues listed above. For the border problem, I've converted Select to use outline like TextField does. Unfortunately, neither Select nor TextField actually fully match the design specs in figma. I think there must've been some changes to the designs since we originally implemented these components. That's probably something to be addressed separately (prob as part of the common comps epic)

Testing

Tested in core app; things generally look right/good (will link associated core PR once that's available)

Screen.Recording.2025-03-24.at.2.34.33.PM.mov

@connieelee connieelee requested review from a team as code owners March 24, 2025 21:06
@circle-job-mirror
Copy link

circle-job-mirror bot commented Mar 24, 2025

@connieelee connieelee force-pushed the connie/select-styles branch from 6cea234 to 4b84302 Compare March 24, 2025 21:17
Comment on lines +131 to +133
if (indicatorProps.isMulti) {
return null;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hides dropdown indicator (chevron icon) for multi-select

@@ -182,7 +206,7 @@ const getStyles = <
},
valueContainer: baseStyles => {
const padding = PADDING[size];
return {...baseStyles, padding};
return {...baseStyles, padding, gap: '2px'};
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adds 2px gap between selected items

Screenshot 2025-03-24 at 2 39 16 PM

@connieelee connieelee changed the title style(ui): Update Select styles to better match TextField style(ui): Update Select component styles Mar 24, 2025
@connieelee connieelee merged commit 20c3df4 into master Mar 24, 2025
138 checks passed
@connieelee connieelee deleted the connie/select-styles branch March 24, 2025 21:49
@github-actions github-actions bot locked and limited conversation to collaborators Mar 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants