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

Global styles affect Percy snapshots #7

Open
ijlee2 opened this issue Jun 24, 2024 · 3 comments
Open

Global styles affect Percy snapshots #7

ijlee2 opened this issue Jun 24, 2024 · 3 comments
Labels
help wanted Extra attention is needed

Comments

@ijlee2
Copy link

ijlee2 commented Jun 24, 2024

Hi. In case you might have missed my feedback in ember-qunit:

I tried out the theme in ember-container-query, and think that Percy failed due to global styles from qunit-theme-ember overriding mine.

Before making qunit-theme-ember the default in v9, can we ensure that:

  • The library doesn't apply styles globally (e.g. set font-family in <body>), but only to the DOM elements for qunit and ember-qunit using ID or class selectors.
  • The CSS variables have a namespace (e.g. --qunit-theme-ember-color-brand) to avoid name collisions.
  • Nice to do: The qunit-theme-ember repo, which is already a monorepo, has a test Ember app with Percy (for continuous integration).
@Techn1x
Copy link

Techn1x commented Jun 25, 2024

Ditto this. Verified in a different visual regression suite (Get Snappy); the fonts from this theme is also being applied to text in my application, which then shows up as a diff in the snapshots.

Keen to switch to it when it is fixed though 😄

@IgnaceMaes
Copy link
Owner

Thanks for reporting, this should be fixed indeed.

Agree we need to do this before thinking about making it the default theme.

@IgnaceMaes IgnaceMaes added the help wanted Extra attention is needed label Jun 29, 2024
@bitxplora
Copy link

bitxplora commented Oct 5, 2024

@IgnaceMaes, it is like the crux is the namespace to make the style distinct and avoid collision.

This means refactoring the variables by prefixing the suggested namespace --qunit-theme-ember to the variables and reflecting it where they are applied will fix this issue. If there is clarity on the issue, I can take it up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants