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

feat: Introduce an API for Percy Specific CSS #346

Merged
merged 8 commits into from
Sep 17, 2019
Merged

Conversation

Robdel12
Copy link
Contributor

@Robdel12 Robdel12 commented Sep 16, 2019

What is this?

This PR introduces first class support for providing CSS specific to only Percy's rendering environment though the SDKs. Now users will be able to provide a new option called percyCSS with our SDKs. This can be set globally and on a per-snapshot basis.

Approach

Initially the implementation of this was going to be fairly easy. We would take a new snapshot option and then serialize that CSS into the DOM when we're serializing inputs and what not. It turns out, CSP can block this injected CSS (thank you SDK test site!).

To work around CSP issues, we instead now take that CSS and create a new resource for this snapshot which contains the CSS. If you have global CSS, we'll concat that with the per-snapshot CSS. This file is named with the current time stamp + percy-specific.css. It does not use the @percy media query since this CSS can only ever exist in Percy)

This CSS will be applied at the very bottom of their DOM snapshot, before the closing </body> tag. This hopefully will ensure the cascade will do its job and !important won't be needed often.

Usage

Now there's a simple API for providing Percy only CSS. For snapshot specific css:

await percySnapshot('Home page', { percyCSS: `iframe { display: none; }` })

For global CSS in the .percy.yml file:

version: 1
snapshot:
  widths: [375, 1280]
  percy-css: | 
    iframe { 
      display none; 
    }

TODOs once this ships

  • Update SDK config docs
  • Update Percy specific CSS docs
  • Ensure all SDKs are passing options correctly.

@@ -36,6 +36,9 @@ jobs:
- run:
name: Client tests
command: $NYC yarn test-client --singleRun
- run:
name: Setup integration test .percy.yml
command: mv .ci.percy.yml .percy.yml
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not crazy about this, but it's needed because adding a .percy.yml file breaks the unit tests.

This can go away once you can pass a custom file path for the .yml file. Then the unit tests can have their own mocked & static .yml file.

@wwilsman
Copy link
Contributor

Haven't reviewed yet, but I just wanted to add that the yml option might be nice to document like this so it's clear how multiple rules might look:

version: 1
snapshot:
  widths: [375, 1280]
  percy-css: |
    iframe {
      display none;
    }

src/services/agent-service.ts Outdated Show resolved Hide resolved
src/services/agent-service.ts Outdated Show resolved Hide resolved
src/services/agent-service.ts Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants