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

CSS Changes to summary #1742

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

CSS Changes to summary #1742

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3003913
CSS changes
abishekTa-egov Nov 5, 2024
ac1bd18
css package update
abishekTa-egov Nov 5, 2024
14423af
Merge branch 'console' into abi2-5-11
abishekTa-egov Nov 5, 2024
739761c
Response backlink change
abishekTa-egov Nov 5, 2024
5e5d7e9
marginBottom corrected
abishekTa-egov Nov 5, 2024
f081919
changes for formulaconfig
abishekTa-egov Nov 5, 2024
43b99f5
pull from console
abishekTa-egov Nov 5, 2024
9093ee9
package update
abishekTa-egov Nov 5, 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 @@ -16,7 +16,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].18/dist/index.css" />
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].19/dist/index.css" />
abishekTa-egov marked this conversation as resolved.
Show resolved Hide resolved

<!-- added below css for hcm-workbench module inclusion-->
<!-- <link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected]/dist/index.css" /> -->
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-health-css",
"version": "0.1.18",
"version": "0.1.19",
"license": "MIT",
"main": "dist/index.css",
"author": "Jagankumar <[email protected]>",
Expand Down
3 changes: 1 addition & 2 deletions health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,11 +118,10 @@
overflow: visible; /* Ensure text isn't cut off */
}

.header-container {
.mp-header-container {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 1rem;
}
Comment on lines +121 to 125
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 adding vertical spacing.

The class renaming and flex layout properties look good. However, consider adding some vertical spacing (margin-bottom) to maintain consistent layout rhythm with the content below.

.mp-header-container {
  display: flex;
  justify-content: space-between;
  align-items: center;
+ margin-bottom: 1rem;
}
📝 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
.mp-header-container {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 1rem;
}
.mp-header-container {
display: flex;
justify-content: space-between;
align-items: center;
margin-bottom: 1rem;
}


.as-table-like {
Expand Down
2 changes: 1 addition & 1 deletion ...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/AssumptionsList.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const AssumptionsList = ({ customProps, setupCompleted }) => {
{Object.keys(dic).map((item, ind) => (
<Card key={`card_${ind}`} style={{ padding: '20px', marginBottom: '15px' }}>
{/* Header with title and edit button */}
<div className="header-container">
<div className="mp-header-container">
abishekTa-egov marked this conversation as resolved.
Show resolved Hide resolved
<HeaderComp title={String(item)} />
{!(setupCompleted === 'true') &&
<Button
Expand Down
4 changes: 2 additions & 2 deletions ...-ui/web/micro-ui-internals/packages/modules/microplan/src/components/DataMgmtComponent.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export const DataMgmtComponent = ({ customProps, setupCompleted }) => {
return (
<>
<Card style={{ marginBottom: "1rem" }}>
<div className="header-container">
<div className="mp-header-container">
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

CSS class name change requires broader updates

The class name change from "header-container" to "mp-header-container" is part of a larger pattern that needs to be consistently applied. The search results show:

  • The new "mp-header-container" class is already being used in multiple microplan module files
  • However, "header-container" is still present in several files across different modules (workbench-hcm, campaign-manager, hcm-microplanning)
  • The CSS is defined in packages/css/src/components/microplan.scss

To maintain consistency:

  • Update remaining instances in microplan module files
  • Consider similar namespace prefixing for other modules using this class
  • Ensure CSS selectors in microplan.scss are updated to match
🔗 Analysis chain

LGTM! Verify CSS cascade impact.

The class name change from "header-container" to "mp-header-container" follows good practice by adding a module-specific prefix. This helps prevent CSS naming collisions.

Let's verify if there are any remaining instances of the old class name that might need updating:

Let me also check for the new class name to understand its usage:

And let's check the specific file content to understand the context better:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any remaining instances of the old class name
rg "header-container" --type js --type jsx

Length of output: 70

Script:

#!/bin/bash
# Search for any remaining instances of the old class name, using ripgrep without file type restrictions
rg "header-container" -l

Length of output: 1324

Script:

#!/bin/bash
# Search for the new class name pattern
rg "mp-header-container" -l

Length of output: 626

Script:

#!/bin/bash
# View the component file content
cat "health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/DataMgmtComponent.js"

Length of output: 5143

<HeaderComp title="POPULATION" styles={{ color: "black" }} />
{!(setupCompleted === 'true') &&
<Button
Expand Down Expand Up @@ -68,7 +68,7 @@ export const DataMgmtComponent = ({ customProps, setupCompleted }) => {
</Card>
<Card>

<div className="header-container">
<div className="mp-header-container">
<HeaderComp title="FACILITIES" styles={{ color: "black" }} />
{!(setupCompleted === 'true') &&

Expand Down
4 changes: 2 additions & 2 deletions ...i/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaConfigScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ const FormulaConfigScreen = ({ customProps, setupCompleted }) => {
{Object.keys(dictionary).length > 0 && (
Object.keys(dictionary).map((category, ind) => (
<Fragment key={category}>
<div className="header-container">
<HeaderComp title={t(String(category))} />
<div className="mp-header-container">
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

Update the remaining .header-container class in campaignCycle.scss

The class name change from header-container to mp-header-container is incomplete. While the new class exists in microplan.scss, the old class name is still being used in:

  • health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/campaignCycle.scss
🔗 Analysis chain

Verify consistent class name updates across stylesheets.

The class name change from header-container to mp-header-container looks good as it helps prevent CSS naming conflicts. Let's verify that the corresponding stylesheet has been updated.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if the old class name still exists in any stylesheets and verify the new class exists
# Expected: No occurrences of the old class name, at least one occurrence of the new class name

echo "Checking for old class name..."
rg -g "*.{css,scss}" "\.header-container"

echo "Verifying new class name exists..."
rg -g "*.{css,scss}" "\.mp-header-container"

Length of output: 459

<HeaderComp title={t(String(category))} styles={{marginBottom:"1rem"}} />
Comment on lines +43 to +44
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

Class name inconsistency found in CampaignBoundary.js

The class name update from 'header-container' to 'mp-header-container' is not consistent across the microplan module. Found one file still using the old class name:

  • health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/CampaignBoundary.js still uses header-container instead of mp-header-container
🔗 Analysis chain

LGTM! Class name update and margin styling look good.

The changes align with the PR objectives:

  • Class name updated to prevent CSS naming conflicts
  • Margin styling properly implemented with correct spelling

Let's verify the class name update is consistent across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify class name updates across the codebase
# Expected: No occurrences of old class name in microplan-related files

# Check for any remaining instances of the old class name in microplan-related files
echo "Checking for old class name in microplan files..."
rg -g "microplan/**" "header-container"

# Verify the new class name exists in stylesheet
echo "Verifying new class name in stylesheets..."
rg -g "*.{css,scss}" "\.mp-header-container"

Length of output: 544

Script:

#!/bin/bash
# Let's try a more accurate search pattern for microplan files
echo "Checking for old class name in microplan files..."
rg -g "**/{microplan,Microplan}/**" "header-container"

# Let's also check the actual implementation of mp-header-container in the stylesheet
echo "Checking mp-header-container implementation..."
rg -A 5 "\.mp-header-container" health/micro-ui/web/micro-ui-internals/packages/css/src/components/microplan.scss

Length of output: 1575

{!(setupCompleted === 'true') &&
<Button
label={t("WBH_EDIT")}
Expand Down
2 changes: 0 additions & 2 deletions .../micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FormulaView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,6 @@ const FormulaView = ({ output="N/A", input1="N/A", input2="N/A", input3="N/A" })
/>
</LabelFieldPair>

{/* Divider */}
<div style={{ background: "#eee", height: "0.2rem", marginBottom: "1rem" }}></div>
</div>
</Card>
</div>
Expand Down
2 changes: 1 addition & 1 deletion ...h/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/HeaderComp.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React from 'react';
import { Card } from '@egovernments/digit-ui-react-components';
import { Header } from '@egovernments/digit-ui-react-components';
import { useTranslation } from 'react-i18next';
const HeaderComp = ({ title,styles = {} }) => {
const HeaderComp = ({ title,styles = {marginBottom:"0rem"} }) => {
abishekTa-egov marked this conversation as resolved.
Show resolved Hide resolved
const {t}=useTranslation();
// Merge default styles with the custom styles passed as a prop
const mergedStyles = { ...styles };
Expand Down
20 changes: 11 additions & 9 deletions health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/Response.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,15 +43,17 @@ const Response = () => {
type="success"
></PanelCard>
<ActionBar className="mc_back">
<Link to={state?.backlink ? state?.backlink : `/${window.contextPath}/employee/`}>
<Button
style={{ margin: "0.5rem", marginLeft: "6rem" }}
className="microplan-response-button"
variation="secondary"
label={t(back)}
icon={"ArrowBack"}
/>
</Link>
<Button
style={{ margin: "0.5rem", marginLeft: "6rem" }}
className="microplan-response-button"
variation="secondary"
label={t(back)}
icon={"ArrowBack"}
onClick={() => {
const backlink = state?.backlink || `/${window.contextPath}/employee/`;
history.push(backlink);
Comment on lines +53 to +54
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 using path.join for backlink construction.

The backlink path construction could be made more robust by using proper path joining.

Consider this improvement:

-const backlink = state?.backlink || `/${window.contextPath}/employee/`;
+const defaultPath = [window.contextPath, 'employee'].join('/');
+const backlink = state?.backlink || `/${defaultPath}`;

This approach:

  • Prevents double slashes
  • Makes path construction more maintainable
  • Is less error-prone
📝 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 backlink = state?.backlink || `/${window.contextPath}/employee/`;
history.push(backlink);
const defaultPath = [window.contextPath, 'employee'].join('/');
const backlink = state?.backlink || `/${defaultPath}`;
history.push(backlink);

}}
/>
Comment on lines +46 to +56
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)

Move inline styles to CSS file for better maintainability.

The Button implementation looks good, but the inline styles should be moved to CSS for better maintainability and consistency.

Consider moving the inline styles to your CSS file:

-<Button
-  style={{ margin: "0.5rem", marginLeft: "6rem" }}
-  className="microplan-response-button"
+<Button
+  className="microplan-response-button back-button"

Then in your CSS:

.back-button {
  margin: 0.5rem;
  margin-left: 6rem;
}

</ActionBar>
</>
);
Expand Down
2 changes: 1 addition & 1 deletion ...icro-ui-internals/packages/modules/microplan/src/components/UserAccessMgmtTableWrapper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ const UserAccessMgmtTableWrapper = ({ role, internalKey,setupCompleted }) => {
{/* <div className="view-composer-header-section">
<CardSubHeader style={{ marginTop: 0, fontSize: "1.5rem", color: " #0B4B66", marginBottom: "0rem" }}>{t(planAssignmentData?.role)}</CardSubHeader>
</div> */}
<div className="header-container">
<div className="mp-header-container">
<HeaderComp title={t(planAssignmentData?.role)} styles={{ color: "black" }} />
{!(setupCompleted === 'true') &&

Expand Down
2 changes: 1 addition & 1 deletion ...ro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/subBoundaryView.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const SubBoundaryView = ({ title, arr, style,editHandler,isEditable}) => {
arr && arr.length > 0 ? (
<Card type={"secondary"} style={style}>

<div className="header-container">
<div className="mp-header-container">
<HeaderComp title={title} />
{isEditable && editHandler && <Button
label={t("WBH_EDIT")}
Expand Down
2 changes: 1 addition & 1 deletion ...-ui/web/micro-ui-internals/packages/modules/microplan/src/pages/employee/SummaryScreen.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ const SummaryScreen = ({ props: customProps }) => {
sections: [
{
type: "DATA",
cardHeader: { value: t("CAMPAIGN_DETAILS"), inlineStyles: { marginTop: 0, marginBottom: "0.5rem", fontSize: "1.5rem" } },
cardHeader: { value: t("CAMPAIGN_DETAILS"), inlineStyles: { marginTop: 0, marginBottom: "1rem", fontSize: "1.5rem" } },
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 moving inline styles to a CSS module or styled-components.

While the margin adjustment is valid, having inline styles scattered throughout the component makes it harder to maintain consistent styling across the application. This is especially important for header styles that should be consistent across different sections.

Consider extracting these styles to a dedicated CSS module or using styled-components:

// styles.module.css
.cardHeader {
  margin-top: 0;
  margin-bottom: 1rem;
  font-size: 1.5rem;
}

// Component
cardHeader: { value: t("CAMPAIGN_DETAILS"), className: styles.cardHeader }

values: [
{
key: t("CAMPAIGN_TYPE"),
Expand Down
2 changes: 1 addition & 1 deletion health/micro-ui/web/public/index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,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" />
<!-- added below css for hcm-workbench module inclusion-->
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].18/dist/index.css" />
<link rel="stylesheet" href="https://unpkg.com/@egovernments/[email protected].19/dist/index.css" />
abishekTa-egov marked this conversation as resolved.
Show resolved Hide resolved
<meta name="viewport" content="width=device-width, initial-scale=1" />
<meta name="theme-color" content="#00bcd1" />
<title>DIGIT HCM</title>
Expand Down