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

Update tag focus and select states to new design #396

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

mkernohanbc
Copy link
Contributor

This PR implements the new offset-ring focus state design on the tag component.

This change makes it easier to distinguish when a tag is in focus, selected or both:

Screenshot 2024-06-25 at 13 55 35

New design in Figma, for reference.

@mkernohanbc mkernohanbc added the enhancement New feature or request label Jun 25, 2024
@mkernohanbc mkernohanbc self-assigned this Jun 25, 2024
@mkernohanbc mkernohanbc linked an issue Jun 25, 2024 that may be closed by this pull request
@mkernohanbc mkernohanbc requested a review from ty2k June 26, 2024 16:12
@ty2k
Copy link
Contributor

ty2k commented Jul 17, 2024

The new focus style looks great!

One existing issue with this component is that our current markup and styles will cause some weirdness for the "Selection" story, as shown in your screenshot. The existing markup and style for the non-selected tag is causing its text and icon to sit visually higher in the container than it should be.

Here's a zoomed in example of what's in this branch currently:
Selected TagGroup using this branch's code

I've added a branch here where I've removed an unnecessary <div> from the markup and simplified the styling. This is what that version of the code looks like:
Selection TagGroup with updated markup

If you open those two screenshots in separate tabs and switch between them, the visual difference should be obvious.

Does that change work for you instead? Can we cherry-pick that commit into this branch?

@mkernohanbc
Copy link
Contributor Author

The new focus style looks great!

One existing issue with this component is that our current markup and styles will cause some weirdness for the "Selection" story, as shown in your screenshot. The existing markup and style for the non-selected tag is causing its text and icon to sit visually higher in the container than it should be.

Here's a zoomed in example of what's in this branch currently: Selected TagGroup using this branch's code

I've added a branch here where I've removed an unnecessary <div> from the markup and simplified the styling. This is what that version of the code looks like: Selection TagGroup with updated markup

If you open those two screenshots in separate tabs and switch between them, the visual difference should be obvious.

Does that change work for you instead? Can we cherry-pick that commit into this branch?

Looks good to me! Merged in 18cf561

@mkernohanbc mkernohanbc merged commit 406341a into main Jul 17, 2024
4 checks passed
@ty2k ty2k deleted the 395-tag---implement-new-selectfocus-state branch July 17, 2024 23:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tag - implement new select/focus state
2 participants