-
Notifications
You must be signed in to change notification settings - Fork 20
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
added checkbox in approved microplan #1987
base: console-develop-v0.3
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe pull request introduces several updates across multiple files. Key changes include updating the stylesheet link in HTML files to a new version of the CSS package, enhancing styling in SCSS files, and modifying the campaign management functionality in JavaScript configurations. Notable additions include a selection handler for UI components and new properties to enable checkboxes in search results. Additionally, a component's configuration for handling row click events has been commented out, altering its behavior. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
⛔ Files ignored due to path filters (1)
health/micro-ui/web/micro-ui-internals/packages/css/package.json
is excluded by!**/*.json
📒 Files selected for processing (6)
health/micro-ui/web/micro-ui-internals/example/public/index.html
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/coreOverride.scss
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/myMicroplanConfig.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ApprovedMicroplans.js
(1 hunks)health/micro-ui/web/public/index.html
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ApprovedMicroplans.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/myMicroplanConfig.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js (1)
Pattern **/*.js
: check
🔇 Additional comments (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ApprovedMicroplans.js (1)
23-27
: Verify the removal of row click functionality
While this change aligns with adding checkbox selection functionality, removing row clicks could impact user experience and accessibility. Please clarify:
- How will users navigate to the detailed view that was previously accessed via row clicks?
- Has keyboard navigation been considered in the new checkbox-only interface?
Let's verify the related changes:
✅ Verification successful
Row click functionality has been intentionally replaced with checkbox selection
The removal of row click functionality is part of a deliberate change to use checkbox selection instead. The code shows:
- The
MicroplanCampaignSearchConfig
in UICustomizations.js has aselectionHandler
that provides the same navigation functionality that was previously handled by row clicks - The ResultsTable component supports both checkbox selection and row clicks through its configuration
- The navigation to detailed view is still preserved, just moved from row clicks to checkbox selection for better user experience
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for checkbox and selection handler implementation
# Check for checkbox configuration
ast-grep --pattern 'MicroplanCampaignSearchConfig = {
$$$
enableCheckbox: true,
$$$
}'
# Look for selection handler implementation
ast-grep --pattern 'selectionHandler = function($_) {
$$$
}'
# Check for alternative navigation methods
rg -A 5 "campaign/fetch-from-microplan"
Length of output: 2205
Script:
#!/bin/bash
# Let's check for any click handlers or navigation methods in the file
rg -A 10 "onClick|onRowClick|navigate|history.push" health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ApprovedMicroplans.js
# Check for any checkbox or selection related code in the file
rg -A 10 "checkbox|select|enableCheckbox|selectionHandler" health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ApprovedMicroplans.js
# Let's also check the search config file for navigation/selection handlers
rg -A 10 "onClick|onRowClick|navigate|history.push|checkbox|select|enableCheckbox|selectionHandler" health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/MicroplanCampaignSearchConfig.js
Length of output: 1515
Script:
#!/bin/bash
# Let's find the correct path for MicroplanCampaignSearchConfig
fd -t f MicroplanCampaignSearchConfig
# Let's also check for any search config files that might contain the checkbox configuration
rg -l "enableCheckbox|selectionHandler" $(fd -t f -e js -e ts -e jsx -e tsx)
Length of output: 65799
Script:
#!/bin/bash
# Let's check the UICustomizations.js file for any alternative navigation methods
cat health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js
# Let's also check the ResultsTable component to understand the checkbox functionality
cat micro-ui/web/micro-ui-internals/packages/react-components/src/hoc/ResultsTable.js
Length of output: 49483
health/micro-ui/web/public/index.html (1)
13-13
: LGTM - Version consistency maintained
The health CSS package version update to 0.2.2 is consistent with the example/public/index.html file.
// additionalConfig={{ | ||
// resultsTable: { | ||
// onClickRow, | ||
// }, | ||
// }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider maintaining both interaction methods
Instead of completely removing row clicks, consider:
- Supporting both checkbox selection and row clicks for different use cases
- Using row clicks for view/details and checkboxes for bulk actions
<InboxSearchComposer
configs={MicroplanCampaignSearchConfig?.[0]}
+ additionalConfig={{
+ resultsTable: {
+ onClickRow: (enableBulkSelect) => !enableBulkSelect && onClickRow,
+ },
+ }}
></InboxSearchComposer>
Committable suggestion skipped: line range outside the PR's diff.
@@ -12,7 +12,7 @@ | |||
<title>DIGIT</title> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-css@1.8.3/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-components-css@0.0.2-beta.42/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-health-css@0.2.1/dist/index.css" /> | |||
<link rel="stylesheet" href="https://unpkg.com/@egovernments/digit-ui-health-css@0.2.2/dist/index.css" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Version inconsistencies detected in CSS package dependencies
The version checks reveal mismatches with the latest published versions:
@egovernments/digit-ui-health-css
: Using 0.2.2 in code but latest is 0.2.1@egovernments/digit-ui-css
: Using 1.8.3 but latest is 1.8.10@egovernments/digit-ui-components-css
: Using 0.0.2-beta.42 but latest is 0.0.2-beta.11
Please:
- Revert health CSS to version 0.2.1 as 0.2.2 appears to be an invalid/unpublished version
- Consider updating other CSS packages to their latest versions for consistency
🔗 Analysis chain
Verify version compatibility across packages
The health CSS package version has been updated to 0.2.2. Please ensure this version is:
- The latest stable release
- Compatible with other CSS packages (@egovernments/digit-ui-css@1.8.3 and digit-ui-components-css@0.0.2-beta.42)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check latest versions of all related packages
echo "Checking latest versions of related packages..."
for package in "@egovernments/digit-ui-health-css" "@egovernments/digit-ui-css" "@egovernments/digit-ui-components-css"; do
echo "\nPackage: $package"
curl -s "https://registry.npmjs.org/$package" | jq -r '.["dist-tags"].latest'
done
Length of output: 1094
.actionBarWrapper { | ||
display: flex ; | ||
justify-content: space-between; | ||
.search-instruction-header{ | ||
margin: 0rem; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
Consider adding responsive styles for actionBarWrapper
The new actionBarWrapper class implements a flex layout with space-between alignment, which is good for desktop views. However, consider adding media queries for mobile responsiveness to ensure proper layout on smaller screens.
.actionBarWrapper {
display: flex;
justify-content: space-between;
+ @media screen and (max-width: 768px) {
+ flex-direction: column;
+ gap: 1rem;
+ }
.search-instruction-header {
margin: 0rem;
}
}
📝 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.
.actionBarWrapper { | |
display: flex ; | |
justify-content: space-between; | |
.search-instruction-header{ | |
margin: 0rem; | |
} | |
.actionBarWrapper { | |
display: flex ; | |
justify-content: space-between; | |
@media screen and (max-width: 768px) { | |
flex-direction: column; | |
gap: 1rem; | |
} | |
.search-instruction-header{ | |
margin: 0rem; | |
} |
selectionHandler: (e) => { | ||
const ids = e.map((item) => item.original?.id).filter(Boolean).join(','); | ||
const microPlanNames = e | ||
.map((item) => item.original?.name.replace(/&/g, "and")) | ||
.filter(Boolean) | ||
.join(','); | ||
const campaignIds = e.map((item) => item.original?.campaignId).filter(Boolean).join(','); | ||
const updatedName = e?.[0]?.original?.name.replace(/&/g, "and"); | ||
window.history.pushState( | ||
{ | ||
microName: microPlanNames, | ||
ids: ids, | ||
campaignID: campaignIds, | ||
}, | ||
"", | ||
`/${window.contextPath}/employee/campaign/fetch-from-microplan?id=${e?.[0]?.original?.campaignId}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplanµName=${updatedName}&planConfigurationId=${e?.[0]?.original?.id}` | ||
); | ||
const navEvent = new PopStateEvent("popstate"); | ||
window.dispatchEvent(navEvent); | ||
|
||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle empty selections and encode URL parameters
The selectionHandler
function assumes that the e
array has at least one element. If e
is empty, accessing e[0]
could lead to errors. Additionally, when constructing URLs, it's best practice to encode URL parameters to handle special characters properly.
Apply this diff to handle empty selections and encode URL parameters:
selectionHandler: (e) => {
+ if (!e || e.length === 0) {
+ return;
+ }
const ids = e.map((item) => item.original?.id).filter(Boolean).join(',');
const microPlanNames = e
- .map((item) => item.original?.name.replace(/&/g, "and"))
+ .map((item) => item.original?.name.replace(/&/g, "and")).map(encodeURIComponent)
.filter(Boolean)
.join(',');
const campaignIds = e.map((item) => item.original?.campaignId).filter(Boolean).join(',');
- const updatedName = e?.[0]?.original?.name.replace(/&/g, "and");
+ const updatedName = encodeURIComponent(e[0].original?.name.replace(/&/g, "and"));
window.history.pushState(
{
microName: microPlanNames,
ids: ids,
campaignID: campaignIds,
},
"",
- `/${window.contextPath}/employee/campaign/fetch-from-microplan?id=${e?.[0]?.original?.campaignId}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplanµName=${updatedName}&planConfigurationId=${e?.[0]?.original?.id}`
+ `/${window.contextPath}/employee/campaign/fetch-from-microplan?id=${encodeURIComponent(e[0].original?.campaignId)}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplanµName=${updatedName}&planConfigurationId=${encodeURIComponent(e[0].original?.id)}`
);
const navEvent = new PopStateEvent("popstate");
window.dispatchEvent(navEvent);
},
📝 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.
selectionHandler: (e) => { | |
const ids = e.map((item) => item.original?.id).filter(Boolean).join(','); | |
const microPlanNames = e | |
.map((item) => item.original?.name.replace(/&/g, "and")) | |
.filter(Boolean) | |
.join(','); | |
const campaignIds = e.map((item) => item.original?.campaignId).filter(Boolean).join(','); | |
const updatedName = e?.[0]?.original?.name.replace(/&/g, "and"); | |
window.history.pushState( | |
{ | |
microName: microPlanNames, | |
ids: ids, | |
campaignID: campaignIds, | |
}, | |
"", | |
`/${window.contextPath}/employee/campaign/fetch-from-microplan?id=${e?.[0]?.original?.campaignId}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplanµName=${updatedName}&planConfigurationId=${e?.[0]?.original?.id}` | |
); | |
const navEvent = new PopStateEvent("popstate"); | |
window.dispatchEvent(navEvent); | |
}, | |
selectionHandler: (e) => { | |
if (!e || e.length === 0) { | |
return; | |
} | |
const ids = e.map((item) => item.original?.id).filter(Boolean).join(','); | |
const microPlanNames = e | |
.map((item) => item.original?.name.replace(/&/g, "and")).map(encodeURIComponent) | |
.filter(Boolean) | |
.join(','); | |
const campaignIds = e.map((item) => item.original?.campaignId).filter(Boolean).join(','); | |
const updatedName = encodeURIComponent(e[0].original?.name.replace(/&/g, "and")); | |
window.history.pushState( | |
{ | |
microName: microPlanNames, | |
ids: ids, | |
campaignID: campaignIds, | |
}, | |
"", | |
`/${window.contextPath}/employee/campaign/fetch-from-microplan?id=${encodeURIComponent(e[0].original?.campaignId)}&draft=true&fetchBoundary=true&draftBoundary=true&source=microplanµName=${updatedName}&planConfigurationId=${encodeURIComponent(e[0].original?.id)}` | |
); | |
const navEvent = new PopStateEvent("popstate"); | |
window.dispatchEvent(navEvent); | |
}, |
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Style
Chores
onClickRow
handler in theInboxSearchComposer
, altering its interaction behavior.