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

refactor(button/icon-button/button-group): style updates #14006

Merged
merged 1 commit into from
Apr 30, 2024

Conversation

SisIvanova
Copy link
Collaborator

@SisIvanova SisIvanova commented Mar 22, 2024

Related to IgniteUI/igniteui-webcomponents/issues/1107

Button Handoff

Button Group Handoff

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

@SisIvanova SisIvanova changed the title refactor(button-group): style updates refactor(button/icon-button/button-group): style updates Mar 26, 2024
@SisIvanova SisIvanova marked this pull request as ready for review April 18, 2024 08:13
@SisIvanova
Copy link
Collaborator Author

@didimmova @AnjiManova Please verify if the width of the buttons in all types is correct.

@didimmova
Copy link
Contributor

didimmova commented Apr 19, 2024

In Bootstrap button group a button with focus-visible + active has foreground of primary-800, I think it should be white. It's triggered with the keyboard by navigating with tab to the desired button and clicking space on it. We should fix it also in the other themes if needed.

Screenshot 2024-04-19 at 15 23 51

didimmova
didimmova previously approved these changes Apr 22, 2024
@AnjiManova
Copy link

@didimmova @AnjiManova Please verify if the width of the buttons in all types is correct.

The widths of all button types look fine to me. (No matter the type, if the content is the same, the widths are equal.)

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

AnjiManova commented Apr 23, 2024

Fluent
I know that the focus in this PR is not Fluent, but it was mentioned that some changes have also been made there, so I checked it.

Button Group
https://github.com/IgniteUI/igniteui-angular/assets/127843837/662996be-a147-4ebf-b2c7-1c0edfd43af4
as we discussed, the focused border shouldn't stay after interaction with the button. Applicable for Light and Dark mode.

Bootstrap
Same thing as in Fluent.

Material
Same thing as in Fluent and Bootstrap.

Same issue in Buttons for all themes.

@SisIvanova SisIvanova added ❌ status: awaiting-test PRs awaiting manual verification and removed ✅ status: verified Applies to PRs that have passed manual verification labels Apr 25, 2024
@SisIvanova
Copy link
Collaborator Author

@didimmova @AnjiManova Sorry for the inconvenience but please verify again the focus-visible / focus-visible + active / focus-visible + hover states in all themes for both button and button group components.

@imincheva
Copy link

imincheva commented Apr 26, 2024

Button Group

  • Material: Min-width of the button group button should be 42px;
  • Fluent: Min-width of the button group button should be 42px;
  • Bootstrap: Min-width of the button group button should be 42px;

@AnjiManova
Copy link

All kits have been reviewed, and there are no comments. Every comment is implemented.

@sbayreva
Copy link

Material

  1. Focused and Hover state of the contained button in Light and Dark Mode - the elevation should be 8

@SisIvanova
Copy link
Collaborator Author

Material

  1. Focused and Hover state of the contained button in Light and Dark Mode - the elevation should be 8
Screenshot 2024-04-30 at 10 02 18 AM Screenshot 2024-04-30 at 10 02 54 AM

The elevation is 8.

@simeonoff simeonoff changed the base branch from master to 17.2.x April 30, 2024 07:43
@simeonoff simeonoff added ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification labels Apr 30, 2024
@simeonoff simeonoff merged commit af32c85 into 17.2.x Apr 30, 2024
3 of 4 checks passed
@simeonoff simeonoff deleted the sivanova/btn-group-styles branch April 30, 2024 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Themes: Update Button and Button Group focus styles
8 participants