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

Jump to bottom

Feature/boundary ss #1490

Open
wants to merge 6 commits into
base: console
Choose a base branch
from
Open

Feature/boundary ss #1490

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
22cc392
boundary-hierarchy
Oct 9, 2024
e5ddd3e
boundary-hierarchy
Oct 9, 2024
1a38522
localization changes
Oct 9, 2024
a33b3a0
version change
Oct 9, 2024
65da08d
ui changes
Oct 10, 2024
b50f660
Update health/micro-ui/web/micro-ui-internals/packages/modules/campai…
jagankumar-egov Oct 10, 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
2 changes: 1 addition & 1 deletion health/micro-ui/web/micro-ui-internals/example/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />

<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].76-campaign/dist/index.css" />
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].77-campaign/dist/index.css" />
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Multiple CSS library versions detected.

Multiple versions of @egovernments/digit-ui-css are linked in the HTML file, which can lead to styling conflicts and inconsistent behaviors. Please consolidate to a single, stable version to ensure consistent styling and improve maintainability.

🔗 Analysis chain

Approved: CSS library version update.

The update from version 1.0.76-campaign to 1.0.77-campaign of @egovernments/digit-ui-css is a minor version change, which typically indicates bug fixes or small improvements. This change is acceptable.

However, please ensure that this update is compatible with the other versions of the CSS library used in this file. Run the following script to list all versions of @egovernments/digit-ui-css being used:

Review the output to confirm that using multiple versions is intentional and doesn't cause any styling conflicts.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: List all versions of @egovernments/digit-ui-css used in the file

echo "Versions of @egovernments/digit-ui-css found in the file:"
rg --no-filename '@egovernments/digit-ui-css@\d+\.\d+\.\d+' health/micro-ui/web/micro-ui-internals/example/public/index.html | sort | uniq

Length of output: 989


<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" />

Expand Down
6 changes: 4 additions & 2 deletions health/micro-ui/web/micro-ui-internals/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@
"*.{js,css,md}": "prettier --write"
},
"dependencies": {
"@egovernments/digit-ui-components": "0.0.2-beta.40",
"@egovernments/digit-ui-react-components": "1.8.2-beta.11",
"ajv": "8.12.0",
"exceljs": "^4.4.0",
"file-saver": "^2.0.5",
"lodash": "4.17.21",
"microbundle-crl": "0.13.11",
"@egovernments/digit-ui-react-components": "1.8.2-beta.11",
"@egovernments/digit-ui-components": "0.0.2-beta.40",
"react": "17.0.2",
"react-dom": "17.0.2",
"react-hook-form": "6.15.8",
Expand Down
2 changes: 1 addition & 1 deletion health/micro-ui/web/micro-ui-internals/packages/css/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@egovernments/digit-ui-css",
"version": "1.0.76-campaign",
"version": "1.0.77-campaign",
"license": "MIT",
"main": "dist/index.css",
"author": "Jagankumar <[email protected]>",
Expand Down
19 changes: 19 additions & 0 deletions health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -952,4 +952,23 @@ tbody {
}
}
}
}.custom-popup-boundary{
max-width: 100%;
height: 11rem;
}
Comment on lines +955 to +958
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 with a suggestion for improvement

The .custom-popup-boundary class effectively sets the maximum width to 100% of its container, which is good for responsiveness. However, the fixed height of 11rem might be limiting for varying content sizes.

Consider using max-height instead of height to allow for more flexible content:

.custom-popup-boundary{
  max-width: 100%;
- height: 11rem;
+ max-height: 11rem;
+ overflow-y: auto;
}

This change would allow the popup to adjust its height based on content, up to a maximum of 11rem, and provide scrolling for overflow content.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
}.custom-popup-boundary{
max-width: 100%;
height: 11rem;
}
.custom-popup-boundary{
max-width: 100%;
max-height: 11rem;
overflow-y: auto;
}





.dustbin-icon{
margin-bottom: 1rem;
margin-top: 0.7rem;
}

.custom-action-bar .digit-action-bar-fields{
display: contents;
}

.digit-action-bar-wrap div {
width: 100%;
}
8 changes: 8 additions & 0 deletions health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,10 @@ import { Wrapper } from "./components/SelectingBoundaryComponent";
import SelectingBoundariesDuplicate from "./components/SelectingBoundariesDuplicate";
import XlsPreview from "./components/XlsPreview";
import BoundarySummary from "./components/BoundarySummary";
import Boundary from "./pages/employee/Boundary";
import GeoPode from "./pages/employee/GeoPode";
import ViewBoundary from "./pages/employee/ViewBoundary";
import ViewHierarchy from "./pages/employee/ViewHierarchy";

import MultiSelectDropdown from "./components/MultiSelectDropdown";
/**
Expand Down Expand Up @@ -133,6 +137,10 @@ const componentsToRegister = {
SelectingBoundariesDuplicate,
XlsPreview,
MultiSelectDropdownBoundary:MultiSelectDropdown,
Boundary,
GeoPode,
ViewBoundary,
ViewHierarchy,
// SelectingBoundaryComponent
BoundarySummary
};
Expand Down
6 changes: 6 additions & 0 deletions ...i/web/micro-ui-internals/packages/modules/campaign-manager/src/components/CampaignCard.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,12 @@ const CampaignCard = () => {
link: "/digit-ui/employee/dss/landing/national-health-dashboard",
roles: ROLES.NATIONAL_SUPERVISOR,
// count: isLoading?"-":data
},
{
label: t("BOUNDARY_MANAGEMENT"),
link: `/${window?.contextPath}/employee/campaign/boundary-management?defaultHierarchyType=HIERARCHYTEST&hierarchyType=DEMOTEST6`,
roles: ROLES.CAMPAIGN_MANAGER,
// count: isLoading?"-":data
Comment on lines +71 to +75
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)

Approve new "Boundary Management" link with suggestion for improvement

The addition of the "Boundary Management" link is approved and aligns with the PR objectives. However, to improve maintainability and flexibility, consider using configuration variables for defaultHierarchyType and hierarchyType instead of hardcoding the values.

Consider refactoring the link construction as follows:

const HIERARCHY_CONFIG = {
  defaultHierarchyType: "HIERARCHYTEST",
  hierarchyType: "DEMOTEST6"
};

// In the links array
{
  label: t("BOUNDARY_MANAGEMENT"),
  link: `/${window?.contextPath}/employee/campaign/boundary-management?defaultHierarchyType=${HIERARCHY_CONFIG.defaultHierarchyType}&hierarchyType=${HIERARCHY_CONFIG.hierarchyType}`,
  roles: ROLES.CAMPAIGN_MANAGER,
}

This approach allows for easier updates to hierarchy types in the future and improves the overall code maintainability.

}
];

Expand Down
69 changes: 69 additions & 0 deletions .../web/micro-ui-internals/packages/modules/campaign-manager/src/components/XlsPreviewNew.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { PopUp, SVG, DownloadIcon, Button } from "@egovernments/digit-ui-react-components";
import React from "react";
import DocViewer, { DocViewerRenderers } from "@cyntler/react-doc-viewer";
import { useTranslation } from "react-i18next";
import { PRIMARY_COLOR } from "../utils";
Comment on lines +1 to +5
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! Consider grouping imports for better organization.

The imports are appropriate for the component's functionality. Good job on using a constant for the primary color, which helps maintain consistency.

Consider grouping the imports by type (e.g., external libraries, internal components, styles) for better organization. For example:

// External libraries
import React from "react";
import { useTranslation } from "react-i18next";
import DocViewer, { DocViewerRenderers } from "@cyntler/react-doc-viewer";

// Internal components
import { PopUp, SVG, DownloadIcon, Button } from "@egovernments/digit-ui-react-components";

// Styles and constants
import { PRIMARY_COLOR } from "../utils";


const ArrowBack = ({ className = "", height = "15", width = "15", styles = {} }) => {
return (
<svg className={className} style={styles} width={width} height={height} viewBox="0 0 15 14" fill="none" xmlns="http://www.w3.org/2000/svg">
<path
d="M14.1663 6.16658H4.02467L8.68301 1.50825L7.49967 0.333252L0.833008 6.99992L7.49967 13.6666L8.67467 12.4916L4.02467 7.83325H14.1663V6.16658Z"
fill={PRIMARY_COLOR}
/>
</svg>
);
};
Comment on lines +7 to +16
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 accessibility attributes to the SVG element.

The ArrowBack component is well-structured and allows for customization. However, there's an accessibility issue with the SVG lacking a title or aria-label.

To improve accessibility, add an aria-label attribute to the SVG element. Here's an example of how you can modify the code:

 return (
   <svg
     className={className}
     style={styles}
     width={width}
     height={height}
     viewBox="0 0 15 14"
     fill="none"
     xmlns="http://www.w3.org/2000/svg"
+    aria-label="Arrow Back"
   >
     <path
       d="M14.1663 6.16658H4.02467L8.68301 1.50825L7.49967 0.333252L0.833008 6.99992L7.49967 13.6666L8.67467 12.4916L4.02467 7.83325H14.1663V6.16658Z"
       fill={PRIMARY_COLOR}
     />
   </svg>
 );

This change will improve the accessibility of the component for users relying on screen readers.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const ArrowBack = ({ className = "", height = "15", width = "15", styles = {} }) => {
return (
<svg className={className} style={styles} width={width} height={height} viewBox="0 0 15 14" fill="none" xmlns="http://www.w3.org/2000/svg">
<path
d="M14.1663 6.16658H4.02467L8.68301 1.50825L7.49967 0.333252L0.833008 6.99992L7.49967 13.6666L8.67467 12.4916L4.02467 7.83325H14.1663V6.16658Z"
fill={PRIMARY_COLOR}
/>
</svg>
);
};
const ArrowBack = ({ className = "", height = "15", width = "15", styles = {} }) => {
return (
<svg
className={className}
style={styles}
width={width}
height={height}
viewBox="0 0 15 14"
fill="none"
xmlns="http://www.w3.org/2000/svg"
aria-label="Arrow Back"
>
<path
d="M14.1663 6.16658H4.02467L8.68301 1.50825L7.49967 0.333252L0.833008 6.99992L7.49967 13.6666L8.67467 12.4916L4.02467 7.83325H14.1663V6.16658Z"
fill={PRIMARY_COLOR}
/>
</svg>
);
};
🧰 Tools
🪛 Biome

[error] 9-9: Alternative text title element cannot be empty

For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.

(lint/a11y/noSvgWithoutTitle)

function XlsPreviewNew({ file, ...props }) {
const { t } = useTranslation();
const documents = file
? [
{
fileType: "xlsx",
fileName: file?.filename,
uri: file?.url,
},
]
: null;

Comment on lines +17 to +28
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)

Consider handling the case when file is null or undefined.

The component checks if file exists before creating the documents array, which is good. However, it might be helpful to add a fallback UI or message when file is null or undefined.

You could add a conditional render like this:

if (!file) {
  return <div>No file selected for preview</div>;
}

This would provide a better user experience when there's no file to display.

return (
<div>
<div style={{ display: "flex", justifyContent: "space-between", marginLeft: "2.5rem", marginRight: "2.5rem", marginTop: "2.5rem" }}>
{/* <Button
label={t("BACK")}
variation="secondary"
icon={<ArrowBack styles={{ height: "1.25rem", width: "1.25rem" }} fill={PRIMARY_COLOR} />}
type="button"
className="workbench-download-template-btn"
onButtonClick={() => props?.onBack()}
/>
<Button
label={t("WBH_DOWNLOAD")}
variation="secondary"
icon={<DownloadIcon styles={{ height: "1.25rem", width: "1.25rem" }} fill={PRIMARY_COLOR} />}
type="button"
className="workbench-download-template-btn"
onButtonClick={() => props?.onDownload()}
/> */}
</div>
Comment on lines +29 to +48
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

Remove or implement commented-out code for navigation buttons.

There's a significant block of commented-out code for navigation buttons. This can lead to confusion and clutter in the codebase.

Either implement the navigation functionality or remove the commented-out code. If this is a work in progress, consider adding a TODO comment explaining why the code is commented out and when it will be implemented.

For example, you could replace the commented-out code with:

// TODO: Implement navigation buttons once the back and download functionalities are ready

This provides context for other developers who might work on this file in the future.

<div className="campaign-popup-module" style={{ marginTop: "1.5rem" }}>
<DocViewer
style={{ height: "80vh", overflowY: "hidden" }}
theme={{
primary: PRIMARY_COLOR,
secondary: "#feefe7",
tertiary: "#feefe7",
textPrimary: "#0B0C0C",
textSecondary: "#505A5F",
textTertiary: "#00000099",
disableThemeScrollbar: true,
}}
documents={documents}
pluginRenderers={DocViewerRenderers}
/>
</div>
Comment on lines +49 to +64
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

Consider extracting inline styles to a separate CSS file or using a CSS-in-JS solution.

The component uses inline styles for layout and dimensions. While this works, it can make the component harder to maintain as it grows.

Consider extracting these styles to a separate CSS file or using a CSS-in-JS solution like styled-components. This would improve the separation of concerns and make the component more maintainable.

For example, you could create a styled component for the DocViewer:

import styled from 'styled-components';

const StyledDocViewer = styled(DocViewer)`
  height: 80vh;
  overflow-y: hidden;
`;

// Then in your render method:
<StyledDocViewer
  theme={{...}}
  documents={documents}
  pluginRenderers={DocViewerRenderers}
/>

This approach would keep your component's JSX cleaner and make it easier to manage styles.

</div>
);
}

export default XlsPreviewNew;
Empty file added ...micro-ui-internals/packages/modules/campaign-manager/src/components/boundaryManagement.js
View file
Open in desktop
Empty file.
65 changes: 65 additions & 0 deletions ...icro-ui-internals/packages/modules/campaign-manager/src/components/icons/ShapeFileIcon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import React from "react";

export const ShpFileIcon = () => (
Copy link
Collaborator

Choose a reason for hiding this comment

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

move to svg library

<svg width="96" height="120" viewBox="0 0 96 120" fill="none" xmlns="http://www.w3.org/2000/svg">
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

Improve SVG accessibility

To enhance accessibility, add a title element or aria-label attribute to the SVG. This provides context for screen readers and improves the user experience for people with disabilities.

Here's a suggested fix:

- <svg width="96" height="120" viewBox="0 0 96 120" fill="none" xmlns="http://www.w3.org/2000/svg">
+ <svg width="96" height="120" viewBox="0 0 96 120" fill="none" xmlns="http://www.w3.org/2000/svg" aria-label="Shape file icon">

Alternatively, you can add a title element as the first child of the SVG:

  <svg width="96" height="120" viewBox="0 0 96 120" fill="none" xmlns="http://www.w3.org/2000/svg">
+   <title>Shape file icon</title>
    {/* ... rest of the SVG content ... */}
  </svg>
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
<svg width="96" height="120" viewBox="0 0 96 120" fill="none" xmlns="http://www.w3.org/2000/svg">
<svg width="96" height="120" viewBox="0 0 96 120" fill="none" xmlns="http://www.w3.org/2000/svg" aria-label="Shape file icon">
🧰 Tools
🪛 Biome

[error] 4-4: Alternative text title element cannot be empty

For accessibility purposes, SVGs should have an alternative text, provided via title element. If the svg element has role="img", you should add the aria-label or aria-labelledby attribute.

(lint/a11y/noSvgWithoutTitle)

{/* File outline */}
<rect
x="5"
y="5"
width="86"
height="110"
rx="10"
ry="10"
fill="url(#gradient)" // Gradient fill
stroke="#DAA520" // Golden outline
strokeWidth="2"
/>

{/* File fold at the top-right corner */}
<path
d="M64 0V24H88"
fill="url(#foldGradient)"
stroke="#DAA520"
strokeWidth="1"
/>

{/* Shadow under the fold */}
<path
d="M64 24L88 24"
stroke="#FFD700"
strokeOpacity="0.5"
strokeWidth="1.5"
/>

{/* Add subtle shadow for depth */}
<rect
x="5"
y="5"
width="86"
height="110"
rx="10"
ry="10"
fill="none"
stroke="rgba(0,0,0,0.2)" // Shadow
strokeWidth="4"
strokeLinecap="round"
/>
Comment on lines +6 to +46
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: SVG shapes and styling are well-implemented

The SVG shapes and their styling are correctly defined, creating a visually appealing icon with depth and gradient effects. The code is also well-commented, which aids in understanding the purpose of each shape.

For consistency, consider using hex color codes throughout instead of mixing with color names. For example, replace #FFD700 with #DAA520 (golden color) where appropriate.


{/* Add .SHP text below the file */}
<text x="28" y="115" fill="#DAA520" fontSize="22" fontWeight="bold">.SHP</text>

{/* Define gradient for file */}
<defs>
<linearGradient id="gradient" x1="0%" y1="0%" x2="100%" y2="100%">
<stop offset="0%" style={{ stopColor: "#FFD700", stopOpacity: 1 }} />
<stop offset="100%" style={{ stopColor: "#FFEA00", stopOpacity: 1 }} />
</linearGradient>

{/* Gradient for the folded corner */}
<linearGradient id="foldGradient" x1="0%" y1="0%" x2="100%" y2="100%">
<stop offset="0%" style={{ stopColor: "#FFDE5A", stopOpacity: 1 }} />
<stop offset="100%" style={{ stopColor: "#FFD700", stopOpacity: 0.8 }} />
</linearGradient>
</defs>
Comment on lines +52 to +63
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: Gradient definitions are correct and enhance the design

The linear gradients for the main file icon and the folded corner are well-defined, adding depth and visual appeal to the icon.

For improved maintainability, consider extracting the color values into constants at the top of the file. This would make it easier to update the color scheme in the future. For example:

const GOLDEN_COLOR = "#DAA520";
const LIGHT_GOLDEN_COLOR = "#FFD700";
const PALE_GOLDEN_COLOR = "#FFEA00";

// Then use these constants in your gradient definitions

</svg>
);
46 changes: 46 additions & 0 deletions .../web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,52 @@ export const UICustomizations = {

},
},
MyBoundarySearchConfig: {
preProcess: (data, additionalDetails) => {
console.log("initiall data is", data);
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

Fix typo in console.log message

There's a typographical error in the console.log message. "initiall" should be corrected to "initial".

Apply this diff to fix the typo:

- console.log("initiall data is", data);
+ console.log("initial data is", data);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log("initiall data is", data);
console.log("initial data is", data);

data.body.BoundaryTypeHierarchySearchCriteria.hierarchyType = data?.state?.searchForm?.Name;
console.log("final data is", data);
Comment on lines +59 to +61
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)

Consider removing console.log statements from production code

There are multiple console.log statements that may not be necessary in production. Excessive logging can clutter the console and potentially expose sensitive information.

Review and remove any debugging statements or wrap them with a condition to ensure they are used appropriately.

Also applies to: 65-65, 69-69, 73-73, 77-77, 83-83

return data;
},
additionalCustomizations: (row, key, column, value, t, searchResult) => {
console.log("the row is", row);
const [isActive, setIsActive] = useState(row?.isActive);
switch (key) {
case "Hierarchy Name":
console.log("hoerarchy name", row?.hierarchyType);
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

Correct typo in console.log messages

The word "hoerarchy" is misspelled in the console.log statements. It should be "hierarchy".

Apply this diff to correct the typos:

- console.log("hoerarchy name", row?.hierarchyType);
+ console.log("hierarchy name", row?.hierarchyType);
- console.log("hoerarchy levels", row?.boundaryHierarchy?.[0]);
+ console.log("hierarchy levels", row?.boundaryHierarchy?.[0]);

Also applies to: 73-73

return row?.hierarchyType;
break;
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

Remove unreachable 'break' statements after 'return'

The break statements after return are unreachable and can be removed to clean up the code.

Apply this diff to remove the unreachable break statements:

case "Hierarchy Name":
  console.log("hierarchy name", row?.hierarchyType);
  return row?.hierarchyType;
- break;
case "Levels":
  console.log("hierarchy levels", row?.boundaryHierarchy?.[0]);
  return row?.boundaryHierarchy?.length;
- break;
case "Creation Date":
  console.log("created time", row?.auditDetails?.createdTime);
  let epoch = row?.auditDetails?.createdTime;
  return Digit.DateUtils.ConvertEpochToDate(epoch);
  // return row?.auditDetails?.createdTime;
- break;

Also applies to: 75-75, 81-81

🧰 Tools
🪛 Biome

[error] 71-71: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

case "Levels":
console.log("hoerarchy levels", row?.boundaryHierarchy?.[0]);
return row?.boundaryHierarchy?.length;
break;
case "Creation Date":
console.log("created time",row?.auditDetails?.createdTime);
let epoch = row?.auditDetails?.createdTime;
return Digit.DateUtils.ConvertEpochToDate(epoch);
// return row?.auditDetails?.createdTime;
break;
Comment on lines +78 to +81
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

Wrap 'case' block in braces to restrict variable scope

The variable epoch is declared inside a case without braces, which can lead to scope leakage. Wrap the case block in braces to properly scope the variable.

Apply this diff to wrap the case block:

case "Creation Date":
+ {
  console.log("created time", row?.auditDetails?.createdTime);
  let epoch = row?.auditDetails?.createdTime;
  return Digit.DateUtils.ConvertEpochToDate(epoch);
  // return row?.auditDetails?.createdTime;
- break;
+ }

Committable suggestion was skipped due to low confidence.

🧰 Tools
🪛 Biome

[error] 78-78: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

[error] 81-81: This code is unreachable

... because this statement will return from the function beforehand

(lint/correctness/noUnreachable)

case "Action":
console.log("download button")
return(
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove logs

<>
<Button
type={"button"}
size={"medium"}
icon={"Add"}
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)

Update the button icon to match the action

The DOWNLOAD button uses the "Add" icon, which might be misleading. Consider using an icon that represents downloading to enhance user experience.

For example, you might use an icon like "Download" if available.

variation={"secondary"}
label={t("DOWNLOAD")}
onClick={()=>{
//setShowPopUp(true);
}}
Comment on lines +92 to +94
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

Implement the 'onClick' handler for the DOWNLOAD button

The onClick handler for the DOWNLOAD button is currently empty. Implement the desired functionality or remove the handler if it's not needed to avoid confusion.

If the functionality is pending, consider adding a TODO comment to indicate that implementation is forthcoming.

/>
</>
)
}

},

},
MyCampaignConfigOngoing: {
preProcess: (data, additionalDetails) => {
const tenantId = Digit.ULBService.getCurrentTenantId();
Expand Down
100 changes: 100 additions & 0 deletions .../micro-ui-internals/packages/modules/campaign-manager/src/configs/boundarySearchConfig.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
export const boundarySearchConfig = [
{
Comment on lines +1 to +2
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)

Consider enhancing code readability

The overall structure of exporting a configuration object is appropriate. However, to improve readability and maintainability, consider the following suggestions:

  1. Add a brief comment at the top of the file explaining the purpose of this configuration.
  2. Consider extracting the configuration object into a separate constant before exporting it. This can make the code more modular and easier to test if needed.

Example:

// Configuration for boundary search functionality
const config = {
  // ... (configuration object)
};

export const boundarySearchConfig = [config];

This approach separates the configuration definition from its export, potentially making it easier to modify or extend in the future.

Also applies to: 98-99

// label: "Search",
type: "search",
apiDetails: {
serviceName: "/boundary-service/boundary-hierarchy-definition/_search",
requestParam: {
"offset": 0,
"limit": 10
},
requestBody: {
BoundaryTypeHierarchySearchCriteria: {
"tenantId": "mz",
Copy link
Collaborator

Choose a reason for hiding this comment

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

tenantid hardcoded

"hierarchyType": ""
},
"offset": 0,
"limit": 10

},
Pagination:{
"offset": 0,
"limit": 10
},
masterName: "commonUiConfig",
moduleName: "MyBoundarySearchConfig",
minParametersForSearchForm: 0,
tableFormJsonPath: "requestParam",
filterFormJsonPath: "requestBody",
searchFormJsonPath: "requestBody",
},
Comment on lines +5 to +30
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

⚠️ Potential issue

Refine API details configuration

The apiDetails section has a few areas that could be improved:

  1. The tenantId is hardcoded to "mz". Consider making this configurable or fetching it dynamically to improve reusability across different tenants.

  2. There's duplication in the pagination parameters. They appear both at the root level and within the Pagination object. Consider consolidating these to avoid potential inconsistencies:

requestBody: {
  BoundaryTypeHierarchySearchCriteria: {
    tenantId: "mz",
    hierarchyType: ""
  },
  Pagination: {
    offset: 0,
    limit: 10
  }
},

  1. The masterName and moduleName fields don't seem to be used in the API call. If they're not necessary, consider removing them to keep the configuration concise.

  2. The minParametersForSearchForm is set to 0, which might allow empty searches. Consider if this is the intended behavior or if a minimum number of parameters should be required.

sections: {
search: {
uiConfig: {
formClassName: "custom-both-clear-search",
primaryLabel: "ES_COMMON_SEARCH",
secondaryLabel: "ES_COMMON_CLEAR_SEARCH",
minReqFields: 0,
fields: [
{
label: "Hierarchy Name",
isMandatory: "false",
key: "Name",
type: "text",
populators: {
name: "Name",
optionsKey: "code",
// options:[]
}
},
// {
// label: "Type of Campaign",
// isMandatory: "false",
// key: "Type",
// type: "text",
// populators: {
// name: "Type",
// optionsKey: "list",
// // options:[]
// }
// },
],
},

show: true
},
searchResult: {
// tenantId: Digit.ULBService.getCurrentTenantId(),
uiConfig: {
columns: [
{
label: "Hierarchy Name",
Copy link
Collaborator

Choose a reason for hiding this comment

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

transalation

jsonpath: "hierarchyType",
additionalCustomization: true
},
{
label: "Levels",
jsonpath: "auditDetails.createdBy",
additionalCustomization: true
},
{
label: "Creation Date",
jsonpath: "auditDetails.createdTime",
additionalCustomization: true
},
{
label: "Action",
jsonPath:"isActive",
additionalCustomization: true
}
],

enableColumnSort: true,
resultsJsonPath: "BoundaryHierarchy"
},
show: true,
},
},
},
];

Loading