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

added checkbox in approved microplan #1987

Open
wants to merge 1 commit into
base: console-develop-v0.3
Choose a base branch
from

Conversation

Bhavya-egov
Copy link
Contributor

@Bhavya-egov Bhavya-egov commented Dec 6, 2024

Summary by CodeRabbit

Release Notes

  • New Features

    • Updated stylesheet for the health UI component to the latest version, enhancing visual consistency.
    • Added checkboxes in the search results table, allowing users to select items and perform actions.
  • Bug Fixes

    • Improved error handling in campaign management functions for better user experience.
  • Style

    • Enhanced styling across various components, including new animations for success messages and improved responsiveness for smaller screens.
  • Chores

    • Commented out the onClickRow handler in the InboxSearchComposer, altering its interaction behavior.

@Bhavya-egov Bhavya-egov requested a review from a team as a code owner December 6, 2024 08:24
Copy link
Contributor

coderabbitai bot commented Dec 6, 2024

📝 Walkthrough

Walkthrough

The 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

File Change Summary
health/micro-ui/web/micro-ui-internals/example/public/index.html Updated stylesheet link from version 0.2.1 to 0.2.2.
health/micro-ui/web/micro-ui-internals/packages/css/src/pages/employee/coreOverride.scss Multiple styling adjustments, including new colors, animations, and layout changes.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/UICustomizations.js Added selectionHandler method and modified retryCampaign function for campaign management.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/configs/myMicroplanConfig.js Added properties showCheckBox and checkBoxActionLabel to enable checkboxes in search results.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/ApprovedMicroplans.js Commented out additionalConfig property affecting row click behavior in InboxSearchComposer.
health/micro-ui/web/public/index.html Updated stylesheet link from version 0.2.1 to 0.2.2.

Possibly related PRs

Suggested reviewers

  • jagankumar-egov

🐇 In the meadow, changes bloom bright,
Styles updated, a lovely sight!
Campaigns now dance with a new flair,
Checkboxes added, selections to share.
With each tweak, our UI takes flight,
Hopping forward, all feels just right! 🌼✨


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

📥 Commits

Reviewing files that changed from the base of the PR and between 5f6239c and 68c96f3.

⛔ 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:

  1. How will users navigate to the detailed view that was previously accessed via row clicks?
  2. 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:

  1. The MicroplanCampaignSearchConfig in UICustomizations.js has a selectionHandler that provides the same navigation functionality that was previously handled by row clicks
  2. The ResultsTable component supports both checkbox selection and row clicks through its configuration
  3. 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.

Comment on lines +23 to +27
// additionalConfig={{
// resultsTable: {
// onClickRow,
// },
// }}
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 maintaining both interaction methods

Instead of completely removing row clicks, consider:

  1. Supporting both checkbox selection and row clicks for different use cases
  2. 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" />
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

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:

  1. The latest stable release
  2. 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

Comment on lines +202 to +207
.actionBarWrapper {
display: flex ;
justify-content: space-between;
.search-instruction-header{
margin: 0rem;
}
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 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.

Suggested change
.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;
}

Comment on lines +359 to +379
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&microName=${updatedName}&planConfigurationId=${e?.[0]?.original?.id}`
);
const navEvent = new PopStateEvent("popstate");
window.dispatchEvent(navEvent);

},
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

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&microName=${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&microName=${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.

Suggested change
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&microName=${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&microName=${updatedName}&planConfigurationId=${encodeURIComponent(e[0].original?.id)}`
);
const navEvent = new PopStateEvent("popstate");
window.dispatchEvent(navEvent);
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant