-
Notifications
You must be signed in to change notification settings - Fork 13.4k
fix(ionic): update colors to import function #29891
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 ↗︎
|
|
|
||
| .input-bottom { | ||
| @include padding(7px, 0); | ||
| @include padding(globals.$ionic-space-100, 0); |
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 adding the ionic theme file to the setContent, the snapshots were trying to add more spacing than expected. After investigating, I noticed that this no longer matched Figma.
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.
With the new changes to Playwright, I realized that the color and font size were not being displayed correctly when using setContent. This was due to the lack of ion-content, ion-content would set these styles.
| --padding-end: #{globals.$ionic-space-400}; | ||
| --padding-bottom: #{globals.$ionic-space-200}; | ||
| --padding-start: #{globals.$ionic-space-400}; | ||
| --color: #{globals.$ionic-color-neutral-1200}; |
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.
Since this color is close to black, the snapshots are going to look very similar.
|
|
||
| :host { | ||
| --background: transparent; | ||
| --color: #{globals.$ionic-color-neutral-1200}; |
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.
Since this color is close to black, the snapshots are going to look very similar.
| } | ||
|
|
||
| :root.ionic { | ||
| --ionic-global-background-color: var(--background); |
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.
If ion-content is not present then the background will be changed to --ionic-global-background-color. The original value is a light brown, which leads to many snapshots needing to be re-generated. To avoid that, the variable is using the white color that comes from --background.
| // -------------------------------------------------- | ||
|
|
||
| :host { | ||
| --color: #{globals.$ionic-color-neutral-800}; |
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.
Updated this to match Figma else it was trying to re-generate with black being the default value.
| <meta charset="UTF-8" /> | ||
| <meta name="viewport" content="width=device-width, initial-scale=1.0, minimum-scale=1.0" /> | ||
| <link href="${baseUrl}/css/ionic.bundle.css" rel="stylesheet" /> | ||
| ${theme === 'ionic' ? `<link href="${baseUrl}/css/ionic/bundle.ionic.css" rel="stylesheet" />` : ''} |
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 matches the script that is being used in the test pages.
|
Closing in favor of #29898 |
Issue number: resolves #
What is the current behavior?
What is the new behavior?
Does this introduce a breaking change?
Other information