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(switch): refine switch theme #14160

Merged
merged 11 commits into from
May 16, 2024

Conversation

didimmova
Copy link
Contributor

Closes #14109

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@desig9stein
Copy link
Contributor

@didimmova

Material
There is an elevation 3 on the thumb on hover, I don't see it in the UI kit. Also, it is triggered upon hovering the thumb itself not the whole switch.

Screen.Recording.2024-04-26.at.7.00.21.mov

Fluent
there is a problem with the keyboard focus
Screenshot 2024-04-26 at 7 09 09

Bootstrap
The outline with keyboard focus should be 4px

UI kit
Screenshot 2024-04-26 at 7 16 06

Implementation
Screenshot 2024-04-26 at 7 16 18

@didimmova
Copy link
Contributor Author

didimmova commented Apr 26, 2024

@didimmova

Material There is an elevation 3 on the thumb on hover, I don't see it in the UI kit. Also, it is triggered upon hovering the thumb itself not the whole switch.

Screen.Recording.2024-04-26.at.7.00.21.mov
Fluent there is a problem with the keyboard focus Screenshot 2024-04-26 at 7 09 09

Bootstrap The outline with keyboard focus should be 4px

UI kit Screenshot 2024-04-26 at 7 16 06

Implementation Screenshot 2024-04-26 at 7 16 18

I fixed the bootstrap and fluent ones. About the material one, the shadow is elevation-1 in all states, so it's static (meaning it doesn't change on hover, so it doesn't matter where you hover) but even if it wasn't, according to the design team, it should be on the thumb, because that's the element you're interacting with and the shadow that's changing lies under it.

Screenshot 2024-04-26 at 8 36 34

@desig9stein desig9stein added ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification labels Apr 26, 2024
@AnjiManova

This comment was marked as resolved.

@imincheva

This comment was marked as resolved.

@AnjiManova

This comment was marked as resolved.

@AnjiManova

This comment was marked as resolved.

@sdimchevski sdimchevski removed their assignment May 15, 2024
@simeonoff simeonoff changed the base branch from master to didimmova/input-controls May 16, 2024 11:35
@simeonoff simeonoff merged commit 2c5f529 into didimmova/input-controls May 16, 2024
1 check failed
@simeonoff simeonoff deleted the didimmova/update-switch branch May 16, 2024 12:28
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.

Switch component refinement
7 participants