-
Notifications
You must be signed in to change notification settings - Fork 13.4k
feat(modal): add default ionic theme styles #29876
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
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
b733f2c to
3ccc987
Compare
| html.ionic ion-modal ion-header ion-toolbar ion-title { | ||
| @include globals.position(0, null, null, 0); | ||
|
|
||
| position: absolute; | ||
|
|
||
| width: 100%; | ||
| height: 100%; | ||
|
|
||
| transform: translateZ(0); | ||
|
|
||
| text-align: center; |
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.
I had to override the styles for ion-title because the ionic theme styles for that component use :host-context() which is supported in Safari and Firefox. I can make this change in the title styles themselves, but it does result in some screenshots changing and I wasn't exactly sure what the correct styles were for that component
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, just minor requests
| @@ -1,6 +1,6 @@ | |||
| @import "./modal.vars"; | |||
| @import "../../themes/native/native.globals"; | |||
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.
Common files do not use any files that are theme associated. I assume that this import was added in order to use mixins like position. If that's the case then import the correct file like how buttons does it:
@use "../../themes/mixins" as mixins;| @@ -1,11 +1,15 @@ | |||
| @use "../../themes/ionic/ionic.globals.scss" as globals; | |||
| @import "./modal"; | |||
| @import "./modal.common"; | |||
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.
| @import "./modal.common"; | |
| @use "./modal.common"; |
| @@ -0,0 +1,49 @@ | |||
| @use "./modal.common"; | |||
| @import "./modal.vars"; | |||
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.
| @import "./modal.vars"; | |
| @use "./modal.vars" as vars |
Let's start to convert over to @use. @import is deprecated so it'll make it easier on us to get in the habit to use @use so we have less areas to convert.
Note: item.ionic.scss has an example of how the vars variable is being used.
Issue number: resolves internal
What is the current behavior?
No default modal styles for the Ionic theme
What is the new behavior?
Does this introduce a breaking change?
Other information
I only applied some of the global styles to the sheet modal vairation since it is unclear whether the card variation will be used in the widgets