-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
✅ Add visual tests for live story notification label #22813
Conversation
Can you also add tests for RTL? |
PTAL. |
Side note: having more tests is always good, but I find it a bit extreme to have 12 screenshots and 3 new files for a simple toast message with 0 parameterization. If we were to do that for every single feature, our visual diff tests would get really really hard to maintain :( Alternative proposal for the next visual diff tests so this message isn't a useless rant:
|
+1 to:
That's behavior, so it seems like a unit test could test this. The visual tests should primarily test the visuals. |
I gave it another go, let me know what you think! |
c74cadf
to
f974b91
Compare
Could you guys also approve the Percy diffs? Thanks! |
* add visual tests for live story notification label * add rtl tests and delete duplicate desktop test * do rtl in same test, remove rtl file, remove behavior logic
Partial for #21714
Adds visual tests for live story notification label, it mocks when a new page is added.