-
Notifications
You must be signed in to change notification settings - Fork 4.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
Tweak appearance of cache page and form #42856
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Graphite Automations"Don't backport" took an action on this PR • (05/20/24)1 reviewer was added and 1 label was added to this PR based on Raphael Krut-Landau's automation. "Notify author when CI fails" took an action on this PR • (05/20/24)1 teammate was notified to this PR based on Raphael Krut-Landau's automation. "Warn authors when publishing large PRs" took an action on this PR • (05/24/24)1 teammate was notified to this PR based on Raphael Krut-Landau's automation. |
|
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 great! Only thing that it would be great to tweak is that we should make hovering on the big rectangles on the left also trigger the :hover
state for the element (making the pill blue and the text white) so that it's clearer that it's a clickable, interactive element.
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.
Looks good overall.
- Please don't use
styles=
prop. - There's a few places where the styling will be much more maintainable if we use fewer styling conventions at once
enterprise/frontend/src/metabase-enterprise/caching/components/StrategyFormLauncher.tsx
Outdated
Show resolved
Hide resolved
enterprise/frontend/src/metabase-enterprise/caching/components/StrategyFormLauncher.tsx
Outdated
Show resolved
Hide resolved
enterprise/frontend/src/metabase-enterprise/caching/components/StrategyFormLauncher.tsx
Outdated
Show resolved
Hide resolved
frontend/src/metabase/admin/performance/components/StrategyEditorForDatabases.styled.tsx
Show resolved
Hide resolved
frontend/src/metabase/admin/performance/components/StrategyForm.tsx
Outdated
Show resolved
Hide resolved
frontend/src/metabase/admin/performance/components/StrategyForm.tsx
Outdated
Show resolved
Hide resolved
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.
❤️
Fixes these issues with the cache page and form:
Loom going over the changes: https://www.loom.com/share/7db77bdc251548bdb93b22101f692147?sid=62e788fe-61a6-4417-9d3d-2f98f18f6431
Closes #42958