-
Notifications
You must be signed in to change notification settings - Fork 13.5k
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(button): add custom properties and remove --ion-color vars #15463
Conversation
@@ -134,6 +212,7 @@ | |||
outline: none; | |||
|
|||
background: var(--background); | |||
color: var(--color); |
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.
could we set the color in the host, and let text-inherit()
do the rest?
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.
We could, but all of the places targeting background
and color
directly are on the button-native
, examples:
// iOS Toolbar Clear Button | ||
// -------------------------------------------------- | ||
|
||
::slotted(*) .button.button-clear .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.
not sure this will work, .button-native is inside the shadow-dom
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.
honestly I think this is safe to remove. I don't even see a difference manually applying the height and there are no differences in the snapshot comparison without it.
references #14808 references #14853 references #14850
📷 📷 📷
Snapshot of this branch: https://ionic-snapshot-go.appspot.com/ionic-core/snapshots/yjw/chrome_400x800
📷 📷 📷
Comparison to master: https://ionic-snapshot-go.appspot.com/ionic-core/snapshots/e1x/yjw/chrome_400x800
Note that I renamed/split the
fill
test tooutline
andclear
since theclear
buttons weren't visually in the window.