Skip to content

fix(button): Trigger changes to the buttongroup when a button is deselected #13813

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
Jan 23, 2024
Merged

fix(button): Trigger changes to the buttongroup when a button is deselected #13813

merged 9 commits into from
Jan 23, 2024

Conversation

@PlamenD95
Copy link
Contributor

@PlamenD95 PlamenD95 commented Jan 16, 2024 •
edited
Loading

Closes #13801

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
@PlamenD95 PlamenD95 requested a review from hanastasov January 16, 2024 14:16
@hanastasov hanastasov added ✅ status: verified Applies to PRs that have passed manual verification and removed ❌ status: awaiting-test PRs awaiting manual verification labels Jan 18, 2024
@hanastasov hanastasov self-requested a review January 18, 2024 08:47
hanastasov
hanastasov previously approved these changes Jan 18, 2024
View reviewed changes
@hanastasov hanastasov self-requested a review January 18, 2024 08:56
@hanastasov
Copy link
Contributor

hanastasov commented Jan 18, 2024

Please add a note in the CHANGELOG about this behavioral change.
hanastasov
hanastasov previously approved these changes Jan 18, 2024
View reviewed changes
@hanastasov
Copy link
Contributor

hanastasov commented Jan 18, 2024

@simeonoff if there is a buttonSelected event it makes sense for buttonDeselected to exist too. The naming would better be selected/deselected but this is another story.

For now, I think going with the same name and not add a new one to the api is ok. One can always base a custom logic based on eventArgs.seelected value.

I am OK with this change, if you are OK too, we we can proceed and merge it.
simeonoff
simeonoff reviewed Jan 22, 2024
View reviewed changes
simeonoff
simeonoff requested changes Jan 22, 2024
View reviewed changes
Copy link
Collaborator

@simeonoff simeonoff left a comment

Choose a reason for hiding this comment

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

See comment.
@simeonoff simeonoff requested a review from rkaraivanov January 22, 2024 09:35
@hanastasov hanastasov self-requested a review January 22, 2024 12:15
@hanastasov hanastasov merged commit 4ffb58b into master Jan 23, 2024
@hanastasov hanastasov deleted the pdobrev/13801-deselecting-button branch January 23, 2024 08:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

button button-group ✅ status: verified Applies to PRs that have passed manual verification

4 participants

@PlamenD95 @hanastasov @simeonoff