-
Notifications
You must be signed in to change notification settings - Fork 19
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
feat: Redesign check type selection screen #957
Conversation
New icons added in grafana/grafana#94439 |
b94c5d3
to
02aac44
Compare
This PR needs Grafana 11.3.0 to be released in order to use the new icons, which is scheduled for 2024-10-22 (as of https://internal-ops-us-east-0.grafana.net/grafana-release-guide/release-schedule/) |
This creates a dependency to @grafana/data 11.3.0 as they've been recently added there.
98c2325
to
372a2fc
Compare
Because of this, we would need to wait for grafana version 11.4.0 in order to use the new icons. And according to this document, version 11.4.0 is scheduled for release on December 5, which feels quite far off. To move forward, I’ve reverted to using the old icons while keeping the rest of the card redesign that Maja proposed. This will allow us to review and merge these changes sooner, and once the new icons are ready, we can easily update them in a new PR. |
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! Code looks good, just a few extra things to address:
I am sure @majavojnoska would like these to default to 2 x 2 when we can't get four cards on the same row.
And similar for when we go down to 1 per row, can we make this prettier, too?
And on the check creation page can we add the accompanying badge to the left of the icon. There is also a bug about where the toggletip is rendering, can you see what is going on there? 🤔
I changed the layout to prevent having 3 columns. According to the breakpoints defined in
I also added the NEW badge in the check form :) |
This looks awesome Vika! ❤️ |
- workaround to fix the bug in floating-ui when setting containerType: inline-size - more info here floating-ui/floating-ui#3067
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.
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.
Last few changes to tighten up 🔧
Thanks Chris! I've applied your suggestions 🙏 |
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.
LGTM 💪
Redesigns the check type selection screen.
Closes #955