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

Feature/boundary ss #1490

Merged
merged 9 commits into from
Oct 15, 2024
Merged

Feature/boundary ss #1490

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
9 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
f54e56b
pr review changes
Oct 13, 2024
1f3b6cb
removed all logs
jagankumar-egov Oct 15, 2024
28d74a9
Merge branch 'console' into feature/boundary-ss
jagankumar-egov Oct 15, 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 +956 to +959
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
55 changes: 55 additions & 0 deletions .../web/micro-ui-internals/packages/modules/campaign-manager/src/components/BoundaryPopup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
import React from "react";
import { PopUp, Button } from "@egovernments/digit-ui-components";
import { useTranslation } from "react-i18next";

const BoundaryPopup = ({ showPopUp, setShowPopUp, callGeoPode, geoPodeData })=> {
const { t } = useTranslation();
return (
showPopUp && (
<PopUp
className={"custom-popup-boundary"}
type={"default"}
heading={t("CHOOSE_MEANS_TO_CREATE_BOUNDARY")}
children={[
]}
onClose={()=>{
setShowPopUp(false);
}}
style={{
height:"11rem",
width: "48rem"
}}
footerChildren={[
]}
sortFooterChildren={true}
>
Comment on lines +8 to +25
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

Remove unnecessary children prop and consider responsive styling.

The PopUp component structure is generally good, but there are two areas for improvement:

  1. The children prop is empty and unnecessary. Remove it to address the static analysis warning.
  2. The style prop uses fixed dimensions, which might not be responsive on different screen sizes.

Consider applying these changes:

  1. Remove the empty children prop:
- children={[
- ]}
  1. Replace fixed dimensions with responsive units:
 style={{
-    height:"11rem",
-    width: "48rem"
+    height: "auto",
+    width: "90%",
+    maxWidth: "48rem"
 }}

This will make the popup more adaptable to different screen sizes while maintaining a maximum width.

📝 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
showPopUp && (
<PopUp
className={"custom-popup-boundary"}
type={"default"}
heading={t("CHOOSE_MEANS_TO_CREATE_BOUNDARY")}
children={[
]}
onClose={()=>{
setShowPopUp(false);
}}
style={{
height:"11rem",
width: "48rem"
}}
footerChildren={[
]}
sortFooterChildren={true}
>
showPopUp && (
<PopUp
className={"custom-popup-boundary"}
type={"default"}
heading={t("CHOOSE_MEANS_TO_CREATE_BOUNDARY")}
onClose={()=>{
setShowPopUp(false);
}}
style={{
height: "auto",
width: "90%",
maxWidth: "48rem"
}}
footerChildren={[
]}
sortFooterChildren={true}
>
🧰 Tools
🪛 Biome

[error] 13-13: Avoid passing children using a prop

The canonical way to pass children in React is to use JSX elements

(lint/correctness/noChildrenProp)

<div style={{display:"flex", gap:"1rem", justifyContent:"space-around"}}>
<Button
type={"button"}
size={"large"}
isDisabled={!geoPodeData}
variation={"secondary"}
label={t("GET_BOUNDARY_DATA_FROM_GEOPODE")}
onClick={() => {
callGeoPode(false);
}}
style={{height:"4rem"}}
/>
<Button
type={"button"}
size={"large"}
variation={"secondary"}
label={t("CREATE_MY_OWN_BOUNDARY_DATA")}
onClick={() => {
callGeoPode(true);
}}
style={{height:"4rem"}}
/>
</div>
Comment on lines +26 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.

🛠️ Refactor suggestion

Good use of internationalization and conditional disabling, but consider removing fixed button heights.

The button container and buttons are well-implemented with good practices:

  • Proper use of internationalization for button labels.
  • Conditional disabling of the first button based on geoPodeData.
  • Good use of flexbox for layout.

However, the fixed height on buttons might not be necessary and could potentially cause issues on different devices or with different text lengths.

Consider removing the fixed height from both buttons:

- style={{height:"4rem"}}

This will allow the buttons to adapt to their content and maintain consistency with the UI library's default styling.

📝 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
<div style={{display:"flex", gap:"1rem", justifyContent:"space-around"}}>
<Button
type={"button"}
size={"large"}
isDisabled={!geoPodeData}
variation={"secondary"}
label={t("GET_BOUNDARY_DATA_FROM_GEOPODE")}
onClick={() => {
callGeoPode(false);
}}
style={{height:"4rem"}}
/>
<Button
type={"button"}
size={"large"}
variation={"secondary"}
label={t("CREATE_MY_OWN_BOUNDARY_DATA")}
onClick={() => {
callGeoPode(true);
}}
style={{height:"4rem"}}
/>
</div>
<div style={{display:"flex", gap:"1rem", justifyContent:"space-around"}}>
<Button
type={"button"}
size={"large"}
isDisabled={!geoPodeData}
variation={"secondary"}
label={t("GET_BOUNDARY_DATA_FROM_GEOPODE")}
onClick={() => {
callGeoPode(false);
}}
/>
<Button
type={"button"}
size={"large"}
variation={"secondary"}
label={t("CREATE_MY_OWN_BOUNDARY_DATA")}
onClick={() => {
callGeoPode(true);
}}
/>
</div>

</PopUp>

)
);
};

export default BoundaryPopup;
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
58 changes: 58 additions & 0 deletions ...-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/FinalPopup.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import React from "react";
import { PopUp, Button } from "@egovernments/digit-ui-components";
import { useTranslation } from "react-i18next";

const FinalPopup = ({ showFinalPopUp, setShowFinalPopup, addParents, createNewHierarchy })=> {
const { t } = useTranslation();
return (
showFinalPopUp && (
<PopUp
className={"custom-popup"}
type={"default"}
heading={t("CREATE_BOUNDARY_HIERARCHY")}
children={[
]}
onClose={()=>{
setShowFinalPopup(false);
}}
onOverlayClick={()=>{
setShowFinalPopup(false);
}}
style={{
// height:"11rem"
width: "50rem"
}}
footerChildren={[
<Button
type={"button"}
size={"large"}
variation={"secondary"}
label={t("CANCEL")}
onClick={() => {
setShowFinalPopup(false);
}}
/>,
<Button
type={"button"}
size={"large"}
variation={"primary"}
label={t("CREATE")}
onClick={() => {
addParents();
createNewHierarchy();
setShowFinalPopup(false);
}}
/>
]}
Comment on lines +25 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.

⚠️ Potential issue

Add key props to Button components in footerChildren array.

While the buttons are implemented correctly, they're missing key props. Since these buttons are part of an array passed to footerChildren, each should have a unique key prop to help React efficiently update the DOM.

Apply this change to both Button components:

 <Button
+    key="cancel-button"
     type={"button"}
     size={"large"}
     variation={"secondary"}
     label={t("CANCEL")}
     onClick={() => {
         setShowFinalPopup(false);
     }}
 />
 <Button
+    key="create-button"
     type={"button"}
     size={"large"}
     variation={"primary"}
     label={t("CREATE")}
     onClick={() => {
         addParents();
         createNewHierarchy();
         setShowFinalPopup(false);
     }}
 />
📝 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
footerChildren={[
<Button
type={"button"}
size={"large"}
variation={"secondary"}
label={t("CANCEL")}
onClick={() => {
setShowFinalPopup(false);
}}
/>,
<Button
type={"button"}
size={"large"}
variation={"primary"}
label={t("CREATE")}
onClick={() => {
addParents();
createNewHierarchy();
setShowFinalPopup(false);
}}
/>
]}
footerChildren={[
<Button
key="cancel-button"
type={"button"}
size={"large"}
variation={"secondary"}
label={t("CANCEL")}
onClick={() => {
setShowFinalPopup(false);
}}
/>,
<Button
key="create-button"
type={"button"}
size={"large"}
variation={"primary"}
label={t("CREATE")}
onClick={() => {
addParents();
createNewHierarchy();
setShowFinalPopup(false);
}}
/>
]}
🧰 Tools
🪛 Biome

[error] 26-34: 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] 35-45: 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)

sortFooterChildren={true}
>
<div>
{<div>{t("YOU_WON'T_BE_ABLE_TO_UNDO_THIS_STEP_OF_CREATING_HIERARCHY")}</div>}
</div>
</PopUp>

)
);
};

export default FinalPopup;
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>
);
Loading