-
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
Home page break issue fix, landing page css fix #1487
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several modifications across different components in the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
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.
Actionable comments posted: 0
🧹 Outside diff range comments (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1)
Line range hint
91-110
: Consider using dynamic icon for LandingPageCard.The
icon
prop inLandingPageCard
is currently set to a hardcoded value "SupervisorAccount". Consider using the icon frommoduleData
for consistency.Here's a suggested fix:
const propsForModuleCard = { - icon: "SupervisorAccount", + icon: moduleData?.icon || "SupervisorAccount", moduleName: t(moduleData?.label), // ... rest of the props };This change will use the icon from
moduleData
if available, falling back to "SupervisorAccount" if not.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
micro-ui/web/micro-ui-internals/packages/css/package.json
is excluded by!**/*.json
📒 Files selected for processing (4)
- micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1 hunks)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/ViewUrl/index.js (1 hunks)
- micro-ui/web/public/index.html (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1)
Pattern
**/*.js
: checkmicro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/ViewUrl/index.js (1)
Pattern
**/*.js
: check
🔇 Additional comments (9)
micro-ui/web/public/index.html (1)
10-10
: CSS package version update looks good.The update of the
@egovernments/digit-ui-css
package from version1.8.2-beta.39
to1.8.2-beta.40
aligns with the PR objective of fixing the landing page CSS. This minor version bump suggests that it likely includes bug fixes or small improvements.To ensure that this CSS update doesn't introduce any breaking changes or unexpected behavior, please run the following verification steps:
After running these verification steps, please ensure:
- The new CSS version exists and is accessible.
- There isn't a significant unexpected change in file size.
- No other files are still referencing the old CSS version.
If any issues are found, please address them before merging this PR.
micro-ui/web/micro-ui-internals/packages/modules/core/src/pages/employee/ViewUrl/index.js (1)
36-36
: LGTM! Verify visual appearance.The change to the SVG.TickMark dimensions (height: 30, width: 70) looks good. This adjustment will make the tick mark shorter and wider, which may improve its visual appearance or fit better within the layout.
Please verify that this change achieves the desired visual effect in the UI. Consider the following:
- Does the new size of the tick mark align well with other elements in the component?
- Is the tick mark clearly visible and not distorted?
- Does this change maintain consistency with similar icons or visual elements across the application?
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (3)
2-2
: LGTM: New imports added correctly.The addition of
Button
andLandingPageWrapper
imports from@egovernments/digit-ui-components
is appropriate for the changes made in the component.
Line range hint
73-89
: LGTM: Improved sorting logic and multi-root tenant handling.The addition of sorting logic for card modules and links based on
mdmsOrderData
enhances the component's flexibility. The condition for applying sorting only in multi-root tenant scenarios is a good optimization.
Line range hint
113-117
: LGTM: Improved component structure with LandingPageWrapper.The use of
LandingPageWrapper
to encapsulate the children improves the layout structure of the component. TheReact.Children.map
withReact.cloneElement
is a good practice for modifying child elements.micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/sandbox.scss (4)
100-100
: Verify the impact of reduced margin on layoutThe bottom margin for
.sandbox-url-footer
has been reduced from 3rem to 1rem. This change might affect the overall layout and spacing of the sandbox component.Please confirm that this change doesn't negatively impact the visual appearance or cause any unintended layout shifts.
Line range hint
435-463
: Review the new home wrapper layoutNew styles have been added for the
.homeWrapper
class, implementing a flex layout for better organization of components. This change improves the layout of the home page, providing a clear separation between the main content and the quick setup component.The implementation looks good, but please ensure that it's responsive and works well on different screen sizes.
Line range hint
701-793
: Nice addition of loader and installation stepsThe new styles for the sandbox loader and installation steps provide a visually appealing and informative user experience during the installation process. The use of animations and transitions enhances the overall look and feel.
Great job on implementing these visual enhancements. They should significantly improve the user experience during the installation process.
Line range hint
1-793
: Overall assessment of sandbox.scssThe changes and additions to this SCSS file appear to enhance the user interface and experience of the sandbox component. The styles are well-organized, follow consistent naming conventions, and incorporate responsive design techniques.
Key improvements include:
- Adjusted layout for the home wrapper
- Addition of visually appealing loader and installation steps
While the changes look good overall, please ensure that:
- All new styles are thoroughly tested across different screen sizes and browsers
- The reduced margin for
.sandbox-url-footer
doesn't negatively impact the layout- The animations and transitions perform well, even on lower-end devices
Great work on improving the sandbox component's styles. These changes should contribute to a better user experience.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes