-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
[UI]: Add Dashboard responsiveness #10980
base: master
Are you sure you want to change the base?
Conversation
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.
@dottharun this is looking good so far.
Charts aside, how are we doing on the Kubernetes dashboards?
Is this still a draft or are you ready for review?
@dottharun, are we tossing in the towel here? |
@leecalcote resp_demo-2024-05-23_11.07.01.mp4 |
@sudhanshutech have you reviewed? |
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
ui/themes/app.js
Outdated
@@ -614,10 +614,18 @@ export const styles = (theme) => ({ | |||
flex: 1, | |||
display: 'flex', | |||
flexDirection: 'column', | |||
[theme.breakpoints.up('sm')]: { | |||
width: `60%`, | |||
overflowX: 'clip', |
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.
@dottharun, confirm that no other page is negatively affected by these broad-sweeping changes, please.
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.
Yes.
- It does break the CSS on other pages, but only when the width is under 600px.
- This needs to be fixed later for an easier path to responsiveness.
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.
i didn't understand @dottharun , there is issue when using this breakpoint on other page right?
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.
Yes.
- Now resolved 2 side-effects caused on other pages.
- Nothing more I could find caused by this PR.
935ffad
to
ae5c933
Compare
Ready for another review. |
067993a
to
270354b
Compare
Latest Demo after the fixes in the designs page: respy_design_demo-2024-05-31_23.55.05.mp4 |
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.
@dottharun please update the PR with the changes regarding padding.
If you need help let me know so that we get this merged quickly. 😁
Signed-off-by: Tharun T <tharun242424@gmail.com>
- padding fix in resourceSubTab Signed-off-by: Tharun T <tharun242424@gmail.com>
- use rem values - fix patternLoadingScreen - fix toolbarOverflow in designs page Signed-off-by: Tharun T <tharun242424@gmail.com>
- by reducing size of buttons - and removing transition from search Signed-off-by: Tharun T <tharun242424@gmail.com>
- fix grid collapse of dashboard overview - fix padding in overrall app's mainContent - fix padding in settings/registry-tabs Signed-off-by: Tharun T <tharun242424@gmail.com>
- by wrapping the modals with a Box with breakpoint widths Signed-off-by: Tharun T <tharun242424@gmail.com>
Signed-off-by: Tharun T <tharun242424@gmail.com>
270354b
to
f25f989
Compare
Signed-off-by: Tharun T <tharun242424@gmail.com>
- for mobile Signed-off-by: Tharun T <tharun242424@gmail.com>
Latest demo after updating padding and modal issues in designs page: respy_pad_mod_demo-2024-06-05_19.16.42.mp4 |
Notes for Reviewers
This PR adds responsiveness to Dashboard UI
Progress
Side effects fix:
designs
page.Note:
Signed commits