Skip to content
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.

Sign up for GitHub

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

how it works and user manual link added in the card #1570

Merged
merged 4 commits into from
Oct 19, 2024
Merged

how it works and user manual link added in the card #1570

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b110249
how it works and user manual link added in the card
mithunhegde-egov Oct 18, 2024
510c4ab
typo
mithunhegde-egov Oct 18, 2024
b8eb25e
removed icons hardcoding and message name logic change
mithunhegde-egov Oct 18, 2024
9af2089
Merge branch 'develop' into how-it-works
mithunhegde-egov Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 70 additions & 14 deletions ...o-ui/web/micro-ui-internals/packages/modules/core/src/components/RoleBasedEmployeeHome.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,17 +69,7 @@ export const RoleBasedEmployeeHome = ({ modules, additionalComponent }) => {
acc[module].links.push({
// 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"
: "PhonelinkSetup",
icon: item.leftIcon,
// link: queryParamIndex === -1 ? linkUrl : linkUrl.substring(0, queryParamIndex),
queryParams: queryParamIndex === -1 ? null : linkUrl.substring(queryParamIndex),
label: t(Digit.Utils.locale.getTransformedLocale(`${module}_LINK_${item.displayName}`)),
Expand Down Expand Up @@ -137,15 +127,72 @@ export const RoleBasedEmployeeHome = ({ modules, additionalComponent }) => {
sortedConfigEmployeesSidebar = configEmployeeSideBar;
}

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
}
});
};
Comment on lines +130 to +152
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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
);
});
};
Comment on lines +154 to +173
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ 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:

  1. Uses an object to define excluded links for each module type.
  2. Simplifies the filtering logic, making it more maintainable.
  3. Allows for easy addition of new module types in the future.


const children = Object.keys(sortedConfigEmployeesSidebar)?.map((current, index) => {
const moduleData = sortedConfigEmployeesSidebar?.[current];
const configureData = moduleData?.links?.find((item) => item?.label === "Configure");
const configureData = moduleData?.links?.find((item) => item?.displayName === "Configure_master");
const howItWorks = getHowItWorksLink(moduleData);
const userManual = getUserManualLink(moduleData);
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()? 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" }}
/>,
],
Comment on lines +185 to +195
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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)

Comment on lines +184 to +195
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

LGTM with minor improvements: Enhanced link filtering and new user manual button

The changes in this segment look good overall:

  1. The use of getFilteredLinks for multi-root tenants enhances the component's functionality.
  2. 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)

endChildren: Digit.Utils.getMultiRootTenant()
? [
<Button
Expand All @@ -157,6 +204,15 @@ export const RoleBasedEmployeeHome = ({ modules, additionalComponent }) => {
onClick={() => history?.push(configureData?.link)}
style={{ padding: "0px" }}
/>,
<Button
variation="teritiary"
label={howItWorks?.label}
icon={howItWorks?.icon}
type="button"
size={"medium"}
onClick={() => window.open(howItWorks?.link, "_blank")}
style={{ padding: "0px" }}
/>,
Comment on lines +207 to +215
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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)

]
: null,
};
Expand Down