-
Notifications
You must be signed in to change notification settings - Fork 1.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
fix(insights): Fix property filter group and expression UI #15758
Conversation
Ahh, Storybook only shows legacy UI, not data exploration… Do you think data exploration is in a state where all of Storybook can have that flag on, or should we still have both branches covered @thmsobrmlr? |
📸 UI snapshots have been updated7 snapshot changes in total. 0 added, 7 modified, 0 deleted:
Triggered by this commit. |
1 similar comment
📸 UI snapshots have been updated7 snapshot changes in total. 0 added, 7 modified, 0 deleted:
Triggered by this commit. |
Yup, it's coming here #15497. The PR should be ready to go apart from a Jest test that I need to fix and go in on Tuesday. Took a while to get this ready since "simply toggling the flag" affected a good chunk of tests and I discovered some issues with Experiments and Notebooks. |
6204545
to
2b06591
Compare
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
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.
✨
frontend/src/lib/components/PropertyFilters/PropertyFilters.scss
Outdated
Show resolved
Hide resolved
9d88b19
to
4f9e74e
Compare
Looks like Firefox visual regression tests have a memory leak and I've found no way to fix this. :/ Will probably remove Firefox in a separate PR. |
Problem
This was a bit ugly:
Changes
This looks better:
How did you test this code?
Added UI snapshots for insight editors.