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

fix: use new button classes, improve conditions and use negative as a variant instead of modifier #62

Merged
merged 1 commit into from
Sep 1, 2023

Conversation

AnnaRybkina
Copy link
Contributor

@AnnaRybkina AnnaRybkina commented Sep 1, 2023

Before After
image image
image image
image image
image image
image
image
image image
image image
image image

@AnnaRybkina AnnaRybkina force-pushed the button-classes-conditions-refactor branch from 60b4755 to 8b73f70 Compare September 1, 2023 06:51
:negative="modifiers.Negative"
:negative="active('Negative')"
Copy link
Contributor

Choose a reason for hiding this comment

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

I support aligning this across framework implementations 🙌 I think we should then have a clear note in the changelog that negative attribute can no longer be used as a modifier that e.g. overrides primary button colors. It could be it was used this way. From now on users will need to replace any other variant that was set on w-button with that negative variant if they want to use it.

Copy link
Contributor Author

@AnnaRybkina AnnaRybkina Sep 1, 2023

Choose a reason for hiding this comment

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

As it is stated in the docs 👇, negative is a modifier for a primary button. Thank you for pointing that. I just fixed how it is on other frameworks but maybe I was too quick:
image

Do you think I should align with the docs and original implementation OR with other frameworks, where negative is a button type. I don't think we need a breaking change for now 😓

Copy link
Contributor

Choose a reason for hiding this comment

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

I think negative should be a button type/variant, so a button can be either primary or negative across all our implementations. There's no examples of Vue Button being migrated from Fabric atm, so we should be good only mentioning this change in the Changelog. I think we might need to avoid adding breaking change in the commit message to not trigger a major version update by semantic release.

@AnnaRybkina AnnaRybkina changed the title fix: use new button classes and improve conditions fix: use new button classes, improve conditions and use negative as a variant instead of modifier Sep 1, 2023
@AnnaRybkina AnnaRybkina merged commit 1c9fa6c into next Sep 1, 2023
5 checks passed
@AnnaRybkina AnnaRybkina deleted the button-classes-conditions-refactor branch September 1, 2023 11:48
github-actions bot pushed a commit that referenced this pull request Sep 1, 2023
## [1.0.1-next.1](v1.0.0...v1.0.1-next.1) (2023-09-01)

### Bug Fixes

* use new button classes and improve conditions ([#62](#62)) ([1c9fa6c](1c9fa6c))
github-actions bot pushed a commit that referenced this pull request Sep 4, 2023
## [1.0.1-next.1](v1.0.0...v1.0.1-next.1) (2023-09-04)

### Bug Fixes

* add test case for negative button as it is not just modifier anymore but it is own button type ([#63](#63)) ([c9d4950](c9d4950))
* use new button classes and improve conditions ([#62](#62)) ([1c9fa6c](1c9fa6c))
github-actions bot pushed a commit that referenced this pull request Sep 18, 2023
## [1.0.1-next.1](v1.0.0...v1.0.1-next.1) (2023-09-18)

### Bug Fixes

* add test case for negative button as it is not just modifier anymore but it is own button type ([#63](#63)) ([c9d4950](c9d4950))
* **buttons:** set secondary variant styles by default ([#67](#67)) ([cde4077](cde4077))
* use new button classes and improve conditions ([#62](#62)) ([1c9fa6c](1c9fa6c))
github-actions bot pushed a commit that referenced this pull request Sep 18, 2023
## [1.0.1-next.1](v1.0.0...v1.0.1-next.1) (2023-09-18)

### Bug Fixes

* add test case for negative button as it is not just modifier anymore but it is own button type ([#63](#63)) ([c9d4950](c9d4950))
* **buttons:** set secondary variant styles by default ([#67](#67)) ([cde4077](cde4077))
* **changelog:** remove changes related to unreleased commits ([#68](#68)) ([364a355](364a355))
* use new button classes and improve conditions ([#62](#62)) ([1c9fa6c](1c9fa6c))
github-actions bot pushed a commit that referenced this pull request Sep 19, 2023
## [1.0.1-next.1](v1.0.0...v1.0.1-next.1) (2023-09-19)

### Bug Fixes

* add test case for negative button as it is not just modifier anymore but it is own button type ([#63](#63)) ([c9d4950](c9d4950))
* **buttons:** set secondary variant styles by default ([#67](#67)) ([cde4077](cde4077))
* **changelog:** remove changes related to unreleased commits ([#68](#68)) ([364a355](364a355))
* update test for a button and changelog ([#69](#69)) ([4eb675e](4eb675e))
* use new button classes and improve conditions ([#62](#62)) ([1c9fa6c](1c9fa6c))
github-actions bot pushed a commit that referenced this pull request Sep 25, 2023
## [1.0.1](v1.0.0...v1.0.1) (2023-09-25)

### Bug Fixes

* Add fullWidth button prop ([#72](#72)) ([f0e76e8](f0e76e8))
* add test case for negative button as it is not just modifier anymore but it is own button type ([#63](#63)) ([c9d4950](c9d4950))
* **buttons:** set secondary variant styles by default ([#67](#67)) ([cde4077](cde4077))
* **changelog:** remove changes related to unreleased commits ([#68](#68)) ([364a355](364a355))
* make tabs accessible for using keyboard keys again ([#71](#71)) ([14129cc](14129cc))
* remove useless test and update changelog ([#70](#70)) ([4ea3fdc](4ea3fdc))
* update test for a button and changelog ([#69](#69)) ([4eb675e](4eb675e))
* use new button classes and improve conditions ([#62](#62)) ([1c9fa6c](1c9fa6c))
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