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.

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

Boundary screen #1462

Open
wants to merge 16 commits into
base: console-develop-v0.3
Choose a base branch
from
Open
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].72-campaign/dist/index.css" />
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].73-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 Versions Detected: Potential for Conflicts

The file health/micro-ui/web/micro-ui-internals/example/public/index.html imports multiple versions of @egovernments/digit-ui-css, including:

  • 1.0.50-microplan
  • 1.0.73-campaign
  • 1.5.41
  • 1.8.0-alpha.6
  • 1.8.2-beta.11

Using multiple CSS versions can lead to style conflicts, increased load times, and maintenance challenges.

Recommended Actions:

  • Consolidate CSS imports to a single, consistent version where possible.
  • Review the necessity of each CSS version and update all imports to the latest stable release to ensure uniform styling across the application.
🔗 Analysis chain

LGTM: CSS version update approved.

The update of the @egovernments/digit-ui-css package from version 1.0.72-campaign to 1.0.73-campaign looks good. This minor version bump likely includes small improvements or bug fixes.

However, I'd like to bring attention to a few points:

  1. Ensure that this CSS update doesn't introduce any unexpected visual changes or breaking styles in the application.
  2. Consider reviewing and potentially consolidating the multiple CSS imports from various sources and versions throughout this file. This could help prevent potential conflicts and improve maintainability.

To verify the impact of this change, you can run the following script:

This script will help ensure that the version update has been applied consistently and provide an overview of all CSS versions in use.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any other occurrences of the old CSS version and list all unique CSS versions used.

# Test 1: Check for any other occurrences of the old CSS version
echo "Checking for other occurrences of the old CSS version:"
rg --type html "1\.0\.72-campaign" health/micro-ui/web/micro-ui-internals/

# Test 2: List all unique CSS versions used
echo -e "\nListing all unique CSS versions used:"
rg --type html -o '@egovernments/digit-ui-css@[0-9.a-z-]+' health/micro-ui/web/micro-ui-internals/ | sort -u

Length of output: 925


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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@egovernments/digit-ui-css",
"version": "1.0.72-campaign",
"version": "1.0.73-campaign",
"license": "MIT",
"main": "dist/index.css",
"author": "Jagankumar <[email protected]>",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,4 +96,17 @@
background-color: #fbe9e6;
border-color: #dc5a32;
}

.subBoundarycomp-container{
display:inline-flex;
flex-wrap:wrap;
gap:0.125rem;
}
Comment on lines +100 to +104
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 minor suggestion for consistency.

The .subBoundarycomp-container class is well-structured for flexible layout of sub-boundary components. The use of inline-flex with flex-wrap allows for responsive arrangement of child elements.

For consistency with other class names in the file, consider using kebab-case:

-.subBoundarycomp-container{
+.sub-boundary-comp-container {
   display: inline-flex;
   flex-wrap: wrap;
   gap: 0.125rem;
 }
📝 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
.subBoundarycomp-container{
display:inline-flex;
flex-wrap:wrap;
gap:0.125rem;
}
.sub-boundary-comp-container {
display: inline-flex;
flex-wrap: wrap;
gap: 0.125rem;
}


.view-more{
text-decoration: underline;
cursor:pointer;
color:orange;
margin-top: 0.2rem;
}
Comment on lines +106 to +111
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

Enhance consistency and user experience for the .view-more class.

The .view-more class effectively styles an interactive "View More" element. However, there are a few suggestions to improve consistency and user experience:

  1. Use a specific hex color instead of "orange" to maintain consistency with the design system.
  2. Add a hover state to provide visual feedback to users.
  3. Consider using em units for margin-top for better scalability.

Here's a suggested improvement:

 .view-more {
   text-decoration: underline;
   cursor: pointer;
-  color: orange;
-  margin-top: 0.2rem;
+  color: #f47738; /* Consistent with other orange elements */
+  margin-top: 0.2em;
+  transition: color 0.3s ease;
+}
+
+.view-more:hover {
+  color: #e26f34; /* Darker shade for hover state */
 }
📝 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
.view-more{
text-decoration: underline;
cursor:pointer;
color:orange;
margin-top: 0.2rem;
}
.view-more {
text-decoration: underline;
cursor: pointer;
color: #f47738; /* Consistent with other orange elements */
margin-top: 0.2em;
transition: color 0.3s ease;
}
.view-more:hover {
color: #e26f34; /* Darker shade for hover state */
}


Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import HeaderComp from "./components/HeaderComp";
import FormulaSection from "./components/FormulaSectionCard";
import FormulaView from "./components/FormulaView";
import SummaryScreen from "./pages/employee/SummaryScreen";
import CampaignBoundary from "./components/CampaignBoundary";

export const MicroplanModule = ({ stateCode, userType, tenants }) => {
const { path, url } = useRouteMatch();
Expand Down Expand Up @@ -60,7 +61,8 @@ const componentsToRegister = {
HeaderComp,
FormulaView,
FormulaSection,
SummaryScreen
SummaryScreen,
CampaignBoundary
Comment on lines +64 to +65
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: CampaignBoundary added to componentsToRegister

The CampaignBoundary component is correctly added to the componentsToRegister object, which allows it to be used within the Microplan module. This addition aligns with the PR objective of enhancing the boundary screen functionality.

Consider removing the extra newline between CampaignBoundary and the closing brace for consistency with the rest of the object:

  SummaryScreen,
- CampaignBoundary
-  
+ CampaignBoundary
};
📝 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
SummaryScreen,
CampaignBoundary
SummaryScreen,
CampaignBoundary
};



};
Expand Down
Loading