-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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
Migrate Box Plot visualization to React #3948
Conversation
…CSS transformations - revert to old solution
Ok I gotta ask - if this visualization is marked as |
I think someone may still use it, therefore we don't drop it. Anyway, it's already migrated (and it didn't took much time)... |
@ranbena @gabrieldutra Updating viewport definitely helped: So now few questions.
WDYT? |
It's actually almost unused. Considering you already did the work to migrate it, then let's keep it. But how about we update the list not to show deprecated types in the list when creating a new visualization but render it if it exists? |
@arikfr It's good idea, but I'll do it in another PR |
In this case I'd say only 1280px is enough. For the default we should think if Mobile Percy diff is that useful in most cases as it doubles the number of screenshots taken on each build. Imo we could leave only the 1280px as a default and trigger mobile ones only for specific pages.
I prefer
🤔 I would do it in case there are Snapshots that require more than only the |
Definitely. |
d18517d
to
71be50c
Compare
Okay, I thinks it's ready for review and (if everyone is happy with code) to merge. |
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.
We're stronger now 💪🏋️♀️
What type of PR is this? (check all applicable)
Description
Related Tickets & Documents
#3301 (Migrate Visualizations to React -> Box Plot)
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
Query page:
Dashboard: