-
Notifications
You must be signed in to change notification settings - Fork 23
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
Stabilize a few UI checks in Chromatic #246
Stabilize a few UI checks in Chromatic #246
Conversation
nickgros
commented
May 19, 2023
•
edited
Loading
edited
- Update action to reference the pull request ref so the actual changes get deployed and tested
- Load Lato font in Storybook preview-head, since it looks like UI tests commonly get flagged as changed when the font doesn't load in time for the screenshot. See https://www.chromatic.com/docs/resource-loading
- Update SubsectionRowRenderer demo to use existing table. The previous table used in the demo no longer exists, and when that happens we show a skeleton that uses a random number to determine its appearance, which is obviously not stable for snapshots.
@@ -1,8 +1,8 @@ | |||
<script> | |||
window.global = window | |||
window.sql = {} |
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.
no longer needed since we removed the sql-parser
package as a dependency
<script | ||
crossorigin | ||
src="https://unpkg.com/react-plotly.js@1.0.2/dist/create-plotly-component.js" | ||
></script> | ||
|
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 is included in our node_modules, we shouldn't need this script.
<script> | ||
const currentStack = JSON.parse( | ||
window.localStorage.getItem('SynapseReactClientStyleGuideStack') ?? 'null', | ||
) | ||
if (currentStack) { | ||
window.SRC = { | ||
OVERRIDE_ENDPOINT_CONFIG: currentStack, | ||
} | ||
} | ||
</script> |
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 was replaced by Storybook controls long ago
- Load Lato font in Storybook preview-head - Update SubsectionRowRenderer demo to use existing table
21f03c7
to
360588f
Compare
@@ -17,6 +17,7 @@ jobs: | |||
steps: | |||
- uses: actions/checkout@v3.1.0 | |||
with: | |||
ref: ${{ github.event.pull_request.head.ref }} |
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.
Deploy the pull request, not the main branch
if (isChromatic && document.fonts) { | ||
loaders.push(fontLoader) | ||
} |
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.
When deployed to Chromatic, wait for the font to load before rendering the story. This should make our screenshots more stable
args: { | ||
sql: 'SELECT funderName as "Funding Agency" FROM syn26344829', |
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 table has been deleted and the fallback is a skeleton with a random length. Update the story with a different table.