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

feat(OnyxSelect): update the hover and focus colors of the chevron icon #2204

Merged
merged 13 commits into from
Dec 4, 2024

Conversation

larsrickert
Copy link
Collaborator

@larsrickert larsrickert commented Dec 2, 2024

Relates to #1818

The colors of the select chevron icon were updated in Figma. I updated them in this PR and also moved the button styles to the shared scss file so we can easily add such buttons other form elements (date picker, inputs etc.) in the future

Checklist

  • The added / edited code has been documented with JSDoc
  • If a new component is added, at least one Playwright screenshot test is added
  • A changeset is added with npx changeset add if your changes should be released as npm package (because they affect the library usage)

@larsrickert larsrickert requested a review from a team as a code owner December 2, 2024 15:24
Copy link

changeset-bot bot commented Dec 2, 2024

🦋 Changeset detected

Latest commit: c18184d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
sit-onyx Minor
demo-app Patch
playground Patch
@sit-onyx/chartjs-plugin Major
@sit-onyx/nuxt Major
@sit-onyx/vitepress-theme Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Collaborator

@JoCa96 JoCa96 left a comment

Choose a reason for hiding this comment

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

To be honest, there is a lot of complexity and edge-cases added just to show a custom icon. Can we skip this until we do the full-scale datepicker implementation? Then the implementation will change anyway.

@JoCa96 JoCa96 self-assigned this Dec 2, 2024
@larsrickert
Copy link
Collaborator Author

To be honest, there is a lot of complexity and edge-cases added just to show a custom icon. Can we skip this until we do the full-scale datepicker implementation? Then the implementation will change anyway.

Where do you seethe complexity in this PR? Imo, the main changes are the screenshots which are only changed because the select chevron color changed in Figma which we need to implement anyways, independent from the date picker.

The date picker just uses the same icon styles as the select

@larsrickert larsrickert force-pushed the feat/1818-datepicker-icon branch from 583da4f to 7f62493 Compare December 3, 2024 10:07
@larsrickert larsrickert changed the title feat(OnyxDatePicker): add custom calendar icon feat(OnyxSelect): update the hover and focus colors of the chevron icon Dec 4, 2024
@larsrickert larsrickert requested a review from JoCa96 December 4, 2024 08:54
larsrickert and others added 13 commits December 4, 2024 10:24
This is an auto-generated pull request. All Playwright screenshots have
been updated.

Co-authored-by: larsrickert <[email protected]>
This is an auto-generated pull request. All Playwright screenshots have
been updated.

Co-authored-by: larsrickert <[email protected]>
This is an auto-generated pull request. All Playwright screenshots have
been updated.

Co-authored-by: larsrickert <[email protected]>
This is an auto-generated pull request. All Playwright screenshots have
been updated.

Co-authored-by: larsrickert <[email protected]>
@larsrickert larsrickert force-pushed the feat/1818-datepicker-icon branch from 83b10fc to c18184d Compare December 4, 2024 09:24
@larsrickert larsrickert enabled auto-merge (squash) December 4, 2024 09:24
@larsrickert larsrickert merged commit 4482d1d into main Dec 4, 2024
19 checks passed
@larsrickert larsrickert deleted the feat/1818-datepicker-icon branch December 4, 2024 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants