-
Notifications
You must be signed in to change notification settings - Fork 13.4k
feat(button): add support for badge #30303
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
Conversation
| .button-native { | ||
| overflow: visible; | ||
|
|
||
| ion-ripple-effect { | ||
| // stylelint-disable-next-line property-disallowed-list | ||
| border-radius: inherit; | ||
| } | ||
|
|
||
| &::after { | ||
| // stylelint-disable-next-line property-disallowed-list | ||
| border-radius: inherit; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After thinking it more, it would be best to revert it to what you had but some changes. Make sure to revert the snapshots as well.
| .button-native { | |
| overflow: visible; | |
| ion-ripple-effect { | |
| // stylelint-disable-next-line property-disallowed-list | |
| border-radius: inherit; | |
| } | |
| &::after { | |
| // stylelint-disable-next-line property-disallowed-list | |
| border-radius: inherit; | |
| } | |
| } | |
| .button-native { | |
| ion-ripple-effect { | |
| // stylelint-disable-next-line property-disallowed-list | |
| border-radius: inherit; | |
| } | |
| &::after { | |
| // stylelint-disable-next-line property-disallowed-list | |
| border-radius: inherit; | |
| } | |
| } | |
| // This rule works for Chrome. | |
| :has(ion-badge) .button-native, | |
| // This rule works for the rest of the browsers. | |
| :host(:has(ion-badge)) .button-native { | |
| --overflow: visible; | |
| } |
# Conflicts: # core/src/components/badge/badge.common.scss
| // This rule works for Chrome. | ||
| :has(ion-badge) .button-native { | ||
| --overflow: visible; | ||
| } | ||
|
|
||
| // This rule works for the rest of the browsers. | ||
| :host(:has(ion-badge)) .button-native { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // This rule works for Chrome. | |
| :has(ion-badge) .button-native { | |
| --overflow: visible; | |
| } | |
| // This rule works for the rest of the browsers. | |
| :host(:has(ion-badge)) .button-native { | |
| // This rule works for Chrome. | |
| :has(ion-badge) .button-native, | |
| // This rule works for the rest of the browsers. | |
| :host(:has(ion-badge)) .button-native { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| // Badge in Button | ||
| // -------------------------------------------------- | ||
|
|
||
| :host(:not(:empty).in-button) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This CSS should be moved to badge.native.scss, along with the vars to badge.native.vars.scss.
Then this css should be repeated on ionic.theme.scss but using the design tokens directly.
This way, wee keep the consistency with the scss architecture of other patterns, between design systems, and if in the future the tokens change values, we won't need to manually change this CSS for the Ionic Theme.
Issue number: internal
What is the current behavior?
The ion-badge is not currently supported when inside the ion-button.
What is the new behavior?
Does this introduce a breaking change?
Other information