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

Dropdown::Toggle::Icon: Fix inconsistencies (HDS-3416) #2178

Merged
merged 9 commits into from
Jun 25, 2024

Conversation

KristinLBradley
Copy link
Contributor

@KristinLBradley KristinLBradley commented Jun 21, 2024

📌 Summary

If merged, this PR fixes inconsistencies in color and toggle icon sizing between the Dropdown::Toggle::Icon as compared to the Button component.

Showcase: https://hds-showcase-git-hds-3416-fix-dropdown-inconsi-809ea9-hashicorp.vercel.app/components/dropdown#icon

📸 Screenshots

BEFORE:

image

AFTER:

image

🔗 External links


👀 Component checklist

💬 Please consider using conventional comments when reviewing this PR.

Copy link

vercel bot commented Jun 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview Jun 25, 2024 4:56pm
hds-website ✅ Ready (Inspect) Visit Preview Jun 25, 2024 4:56pm

@KristinLBradley KristinLBradley marked this pull request as ready for review June 21, 2024 21:25
@KristinLBradley KristinLBradley requested review from a team and jorytindall June 21, 2024 21:25
@jorytindall

This comment was marked as resolved.

Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

I added a couple of other comments on Friday, but I failed to submit the review 😞 .

I've also added a bit of a matrix here for what is changing. Let me know if this makes sense, but essentially I think it breaks down like this:

  • Dropdown::ToggleIcon where hasChevron={{true}} and has an icon
    • size="small" icon size should be 12px
    • size="medium" icon size should 16px

I think everything else should be unchanged.

Copy link
Contributor

@jorytindall jorytindall left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for the work iterating here!

@KristinLBradley KristinLBradley changed the title Dropdown: Fix inconsistencies (HDS-3416) Dropdown::Toggle::Icon: Fix inconsistencies (HDS-3416) Jun 25, 2024
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

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

Left a few suggestions, but nothing is a blocker.

PS: for the record: I think the 12px icon size is too small, but that's a design decision.

packages/components/src/styles/components/dropdown.scss Outdated Show resolved Hide resolved
packages/components/src/styles/components/dropdown.scss Outdated Show resolved Hide resolved
packages/components/src/styles/components/dropdown.scss Outdated Show resolved Hide resolved
packages/components/src/styles/components/dropdown.scss Outdated Show resolved Hide resolved
Co-authored-by: Cristiano Rastelli <cristiano.rastelli@hashicorp.com>
@KristinLBradley KristinLBradley merged commit 7696f53 into main Jun 25, 2024
15 checks passed
@KristinLBradley KristinLBradley deleted the hds-3416-fix-dropdown-inconsistencies branch June 25, 2024 17:22
@hashibot-hds hashibot-hds mentioned this pull request Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants