Improve developer experience to avoid regressing visual components with many variations
Categories
(Firefox :: New Tab Page, enhancement, P3)
Tracking
()
People
(Reporter: Mardak, Unassigned)
References
Details
As noted in bug 1531099, there have been CSS related regressions, but a larger picture is more generally making sure component changes don't break things and do what they should do in all variations. It was noted that dev-test-all is relatively comprehensive, but it's not a great experience in making sure "for all Lists, did changing X not break 1) sizes: 1 vs 2 vs 3 columns 2) images or not 3) numbered or not, etc." as they're scattered throughout a large page.
dmose would probably suggest Storybook
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
There are various things in this space, a web search will find them. As I recall, there was something :jlast liked even better than Storybook (am I thinking of Percy or Cypress or something else?).
Storybook is pretty good for this, seems to have the most mind-share, and now has the advantage that it's useful for non-React stuff too (eg pure CSS). Note that it will get us that coverage for manual development as well as for manual design feedback during development; if we want to do automated visual diffing, we'd probably need to do some work on top of it, or else to pay for a service that runs Storybook tests in CI (eg Chromatic, Percy, ...).
Comment 2•5 years ago
|
||
Er, s/pure CSS/pure HTML/.
Updated•5 years ago
|
Updated•5 years ago
|
Comment 3•5 years ago
|
||
Storybook is great. I also think the percy integration for automated visual testing is really helpful.
Updated•5 years ago
|
Comment 4•5 years ago
|
||
I've hacked together a prototype of using Storybook for a few DiscoveryStream components. An example of what this looks like is at https://dmose.github.io/activity-stream/.
The PR is at https://github.com/mozilla/activity-stream/pull/4923/files, and it's failing to pass the CI only because I didn't set it up with credentials that would allow it to push to master. Doing that cleanly would be something like
Left as an exercise for the next owner of this bug are:
-
securely making the credentials available to push this automatically during CI; https://wiki.mozilla.org/GitHub and https://wiki.mozilla.org/GitHub/Repository_Security are likely to be relevant here. Note, however, that we don't necessarily have to push the Storybook static build into the same repo, and the storybook-deployer allows it to be pushed to S3, which might end up making it easier.
-
making a link to the storybook for any built commit in the PR appear in the PR page.
Worth spinning out to other bugs:
-
making the (Travis) build turn red if visual regressions are found. https://storybook.js.org/docs/testing/automated-visual-testing/ points to lots of possible avenues here, including Percy, as mentioned above by Jason.
-
making Treeherder (Tier ?) turn red if visual regressions are found.
-
making Phabricator deploy Storybook on pushes to m-c
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Assignee | ||
Updated•5 years ago
|
Reporter | ||
Updated•5 years ago
|
Updated•4 years ago
|
Updated•2 years ago
|
Description
•