how it works and user manual link added in the card #1570
Conversation
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested reviewers
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1)
Pattern
**/*.js: check
🪛 Biome
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js
[error] 154-154: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 155-163: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 176-184: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (3)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (3)
82-85: LGTM: New icon selections added consistently.The new icon selections for user manuals and functional specifications have been added consistently with the existing pattern. This improves the visual representation of different types of links.
153-153: LGTM: Appropriate filtering for multi-root tenants.The
linksarray is now correctly filtered to exclude "How it Works" and "Read User Manual" links for multi-root tenants, which aligns with the PR objectives.
Line range hint
1-200: Overall implementation looks good with minor improvements needed.The changes successfully implement the new "How it works" and "Read User Manual" buttons, and update the links array as intended. The code is consistent with the existing patterns and achieves the PR objectives.
To improve the implementation:
- Add
keyprops to React elements in arrays as suggested in the previous comments.- Consider adding error handling for cases where
userManualorhowItWorksmight be undefined.Great job on implementing these new features!
🧰 Tools
🪛 Biome
[error] 154-154: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 155-163: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1)
Pattern
**/*.js: check
🪛 Biome
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js
[error] 154-154: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 155-163: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 176-184: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (3)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (3)
82-85: LGTM: Icon selection logic extended appropriately.The new conditions for "user-manual" and "functional-specifications" links have been added consistently with the existing pattern. This change aligns well with the PR objectives of adding new functionalities.
147-148: LGTM: New variables added for "How it works" and "Read User Manual" links.The new variables
howItWorksanduserManualare correctly implemented to find the respective links in themoduleData.linksarray. This change supports the PR objectives of adding new functionalities.
153-153: LGTM: Links filtering updated for multi-root tenants.The
linksproperty has been correctly updated to filter out "Configure", "How it works", and "Read User Manual" links for multi-root tenants. This change ensures these links are handled separately and don't appear in the module card links for multi-root tenants, aligning with the PR objectives.
| centreChildren: [<div>{t(Digit.Utils.locale.getTransformedLocale(`MODULE_CARD_DESC_${current}`))}</div>, | ||
| <Button | ||
| variation="teritiary" | ||
| label={userManual?.label} | ||
| icon={userManual?.icon} | ||
| type="button" | ||
| size={"medium"} | ||
| onClick={() => window.open(userManual?.link, "_blank")} | ||
| style={{ padding: "0px" }} | ||
| />, | ||
| ], |
There was a problem hiding this comment.
Add key prop to React elements in centreChildren array.
The new button for the user manual has been added correctly and aligns with the PR objectives. However, React elements in an array should have a unique key prop to help React identify which items have changed, been added, or been removed.
Please add a unique key prop to each element in the centreChildren array. For example:
centreChildren: [
<div key="description">{t(Digit.Utils.locale.getTransformedLocale(`MODULE_CARD_DESC_${current}`))}</div>,
<Button
key="userManual"
variation="teritiary"
label={userManual?.label}
icon={userManual?.icon}
type="button"
size={"medium"}
onClick={() => window.open(userManual?.link, "_blank")}
style={{ padding: "0px" }}
/>,
],🧰 Tools
🪛 Biome
[error] 154-154: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 155-163: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
| const propsForModuleCard = { | ||
| icon: "SupervisorAccount", | ||
| moduleName: t(moduleData?.label), | ||
| metrics: [], | ||
| links: Digit.Utils.getMultiRootTenant() ? moduleData.links?.filter((item) => item.label !== "Configure") : moduleData.links, | ||
| centreChildren: [<div>{t(Digit.Utils.locale.getTransformedLocale(`MODULE_CARD_DESC_${current}`))}</div>], | ||
| links: Digit.Utils.getMultiRootTenant() ? moduleData.links?.filter((item) => item.label !== "Configure" && item.label !== "How it works" && item.label !== "Read User Manual") : moduleData.links, |
There was a problem hiding this comment.
why directly add the message ?
u will have to compare the message only
| @@ -79,6 +79,10 @@ export const RoleBasedEmployeeHome = ({ modules, additionalComponent }) => { | |||
| ? "Edit" | |||
| : linkUrl.includes("dss") | |||
| ? "Dashboard" | |||
| : linkUrl.includes("user-manual") | |||
There was a problem hiding this comment.
clean it up
// link: linkUrl,
link: linkUrl,
icon: linkUrl.includes("create")
? "Person"
: linkUrl.includes("inbox")
? "AllInbox"
: linkUrl.includes("search")
? "Search"
: linkUrl.includes("edit")
? "Edit"
: linkUrl.includes("dss")
? "Dashboard"
: linkUrl.includes("user-manual")
? "SpeakerNotes"
: linkUrl.includes("functional-specifications")
? "Book"
: "PhonelinkSetup",
it should be separate master
There was a problem hiding this comment.
downt have hardcoded into system
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (1)
Pattern
**/*.js: check
🪛 Biome
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js
[error] 185-185: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 186-194: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 207-215: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (2)
micro-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js (2)
Line range hint
72-77: LGTM: Simplified icon assignment and code cleanupThe changes in this segment look good. The direct assignment of
item.leftIconto theiconproperty simplifies the code. The removal of commented-out code forlinkimproves code cleanliness.
177-179: LGTM: Enhanced link retrievalThe changes in this segment look good. The
configureDatavariable now correctly finds the "Configure_master" link. The introduction ofhowItWorksanduserManualvariables, using the newly defined functions, enhances the component's functionality and aligns with the PR objectives.
| const getHowItWorksLink = (moduleData) => { | ||
| return moduleData?.links?.find((item) => { | ||
| if (moduleData.module === "PGR") { | ||
| return item?.displayName === "HOW_IT_WORKS_PGR"; | ||
| } else if (moduleData.module === "HRMS") { | ||
| return item?.displayName === "HOW_IT_WORKS_HRMS"; | ||
| } else { | ||
| return false; | ||
| } | ||
| }); | ||
| }; | ||
|
|
||
| const getUserManualLink = (moduleData) => { | ||
| return moduleData?.links?.find((item) => { | ||
| if (moduleData.module === "PGR") { | ||
| return item?.displayName === "PGR_INFO_LINK"; | ||
| } else if (moduleData.module === "HRMS") { | ||
| return item?.displayName === "HRMS_INFO_LINK"; | ||
| } else { | ||
| return false; // No match for other modules | ||
| } | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: New functions for retrieving specific links
The introduction of getHowItWorksLink and getUserManualLink functions is a good addition. They effectively retrieve specific links based on the module type, aligning with the PR objectives.
Consider using a switch statement or object lookup for better scalability if more modules are expected in the future. For example:
const getHowItWorksLink = (moduleData) => {
const howItWorksMap = {
PGR: "HOW_IT_WORKS_PGR",
HRMS: "HOW_IT_WORKS_HRMS"
};
return moduleData?.links?.find(item => item?.displayName === howItWorksMap[moduleData.module]);
};This approach would make it easier to add new modules in the future.
| const getFilteredLinks = (moduleData) => { | ||
| return moduleData.links?.filter((item) => { | ||
| const displayName = item.displayName; | ||
| const isPGR = moduleData.module === "PGR"; | ||
| const isHRMS = moduleData.module === "HRMS"; | ||
| const isNotConfigureMaster = displayName !== "Configure_master"; | ||
| const isNotHowItWorksPGR = !(isPGR && displayName === "HOW_IT_WORKS_PGR"); | ||
| const isNotHRMSInfoLink = !(isHRMS && displayName === "HRMS_INFO_LINK"); | ||
| const isNotHowItWorksHRMS = !(isHRMS && displayName === "HOW_IT_WORKS_HRMS"); | ||
| const isNotPGRInfoLink = !(isPGR && displayName === "PGR_INFO_LINK"); | ||
|
|
||
| return ( | ||
| isNotConfigureMaster && | ||
| isNotHowItWorksPGR && | ||
| isNotHRMSInfoLink && | ||
| isNotHowItWorksHRMS && | ||
| isNotPGRInfoLink | ||
| ); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor suggestion: Simplify link filtering logic
The getFilteredLinks function effectively filters out unwanted links. However, the current implementation could be simplified for better readability and maintainability. Consider refactoring the function as follows:
const getFilteredLinks = (moduleData) => {
const excludedLinks = {
PGR: ["Configure_master", "HOW_IT_WORKS_PGR", "PGR_INFO_LINK"],
HRMS: ["Configure_master", "HRMS_INFO_LINK", "HOW_IT_WORKS_HRMS"],
default: ["Configure_master"]
};
const excludeList = excludedLinks[moduleData.module] || excludedLinks.default;
return moduleData.links?.filter(item => !excludeList.includes(item.displayName));
};This refactored version:
- Uses an object to define excluded links for each module type.
- Simplifies the filtering logic, making it more maintainable.
- Allows for easy addition of new module types in the future.
| links: Digit.Utils.getMultiRootTenant()? getFilteredLinks(moduleData): moduleData.links, | ||
| centreChildren: [<div>{t(Digit.Utils.locale.getTransformedLocale(`MODULE_CARD_DESC_${current}`))}</div>, | ||
| <Button | ||
| variation="teritiary" | ||
| label={userManual?.label} | ||
| icon={userManual?.icon} | ||
| type="button" | ||
| size={"medium"} | ||
| onClick={() => window.open(userManual?.link, "_blank")} | ||
| style={{ padding: "0px" }} | ||
| />, | ||
| ], |
There was a problem hiding this comment.
LGTM with minor improvements: Enhanced link filtering and new user manual button
The changes in this segment look good overall:
- The use of
getFilteredLinksfor multi-root tenants enhances the component's functionality. - The new user manual button is correctly implemented and aligns with the PR objectives.
However, there's a minor issue to address:
Add key props to the React elements in the centreChildren array to help React identify which items have changed, been added, or been removed. For example:
centreChildren: [
<div key="description">{t(Digit.Utils.locale.getTransformedLocale(`MODULE_CARD_DESC_${current}`))}</div>,
<Button
key="userManual"
variation="teritiary"
label={userManual?.label}
icon={userManual?.icon}
type="button"
size={"medium"}
onClick={() => window.open(userManual?.link, "_blank")}
style={{ padding: "0px" }}
/>,
],🧰 Tools
🪛 Biome
[error] 185-185: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 186-194: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
| <Button | ||
| variation="teritiary" | ||
| label={howItWorks?.label} | ||
| icon={howItWorks?.icon} | ||
| type="button" | ||
| size={"medium"} | ||
| onClick={() => window.open(howItWorks?.link, "_blank")} | ||
| style={{ padding: "0px" }} | ||
| />, |
There was a problem hiding this comment.
LGTM with minor improvements: New "How it works" button added
The addition of the "How it works" button to the endChildren array is a good improvement and aligns with the PR objectives. The button is correctly implemented to open the link in a new tab.
However, there's a minor issue to address:
Add key props to the React elements in the endChildren array to help React identify which items have changed, been added, or been removed. For example:
endChildren: Digit.Utils.getMultiRootTenant()
? [
<Button
key="configure"
variation="teritiary"
label={configureData?.label}
icon={configureData?.icon}
type="button"
size={"medium"}
onClick={() => history?.push(configureData?.link)}
style={{ padding: "0px" }}
/>,
<Button
key="howItWorks"
variation="teritiary"
label={howItWorks?.label}
icon={howItWorks?.icon}
type="button"
size={"medium"}
onClick={() => window.open(howItWorks?.link, "_blank")}
style={{ padding: "0px" }}
/>,
]
: null,🧰 Tools
🪛 Biome
[error] 207-215: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
added how it works and user manual. extended the same logic for new url and added the buttons in the array. data and role actions in mdms is udpated
Summary by CodeRabbit