-
Notifications
You must be signed in to change notification settings - Fork 11
** ON HOLD ** Resolved Bug 523 : Design System > Implement responsiveness for Layout, App Shell, Charts #2029
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
base: development
Are you sure you want to change the base?
Conversation
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…lement-responsiveness-for-Layout-App-Shell-Charts
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.
Pull Request Overview
This PR implements responsiveness improvements for the Layout, App Shell, and Charts by updating responsive styles and adjusting component spacing. Key changes include:
- Updating the SCSS for the responsive sidebar icon by removing the !important flag.
- Adjusting logo spacing and visibility in the top navigation with added margin and responsive display utility classes.
- Modifying padding on the right-side menu to better align with the updated layout.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
raaghu-react-themes/src/styles/responsive.scss | Removed !important flag from the sidebar icon’s content property to update specificity. |
raaghu-components/src/rds-comp-top-navigation/rds-comp-top-navigation.tsx | Adjusted logo and text element classes to include additional spacing and responsive display classes. |
raaghu-components/src/rds-comp-top-navigation/rds-comp-top-navigation.tsx (right menu update) | Increased horizontal padding on the right-side menu for improved layout balance. |
Comments suppressed due to low confidence (3)
raaghu-react-themes/src/styles/responsive.scss:160
- Removing the !important flag may reduce specificity and affect overriding behavior. Verify that this change does not introduce style conflicts in the responsive design.
content: url("data:image/svg+xml,%3csvg xmlns=%27http://www.w3.org/2000/svg%27 width=%2712%27 height=%2712%27 viewBox=%270 0 16 16%27%3e%3cpath fill=%27none%27 stroke=%27%23343a40%27 stroke-linecap=%27round%27 stroke-linejoin=%27round%27 stroke-width=%272%27 d=%27M2 5l6 6 6-6%27/%3e%3c/svg%3e")
raaghu-components/src/rds-comp-top-navigation/rds-comp-top-navigation.tsx:1991
- The addition of margin classes (ms-4 and ps-5) changes the logo's positioning; ensure that these spacing adjustments align with the overall responsive layout standards.
className="cursor-pointer sidenav-mobile-logo ms-4 ps-5"
raaghu-components/src/rds-comp-top-navigation/rds-comp-top-navigation.tsx:2039
- Increasing horizontal padding from 2 to 5 may affect layout balance; ensure this change is consistent with the overall design system's spacing guidelines.
"d-flex px-5 align-items-center justify-content-between right-side-menu"
@@ -2008,13 +2008,13 @@ const filterMenuItem = (menuItem: { label: string, children?: any[] }, query: st | |||
{(!props.product1 && <div> | |||
{props.showLogo && ( | |||
<img | |||
className="cursor-pointer pe-4" | |||
className="cursor-pointer pe-4 d-md-inline d-none" |
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.
Consider reordering the display utility classes to 'd-none d-md-inline' for clarity and consistency with Bootstrap conventions.
className="cursor-pointer pe-4 d-md-inline d-none" | |
className="cursor-pointer pe-4 d-none d-md-inline" |
Copilot uses AI. Check for mistakes.
width={140} | ||
src={brandLogo} | ||
alt="raaghu-logo" | ||
></img> | ||
)} | ||
{((!props.product4 && !props.entertainment1)&& <span className="text-bold text-primary ps-4"> | ||
{((!props.product4 && !props.entertainment1)&& <span className="text-bold text-primary d-none d-md-inline ps-4"> |
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.
Consider reordering the display utility classes to 'd-none d-md-inline' for improved clarity and to align with standard Bootstrap responsive patterns.
Copilot uses AI. Check for mistakes.
Description
Resolved Implement responsiveness for Layout, App Shell, Charts
Screenshots:
....*....
Checklist: