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

HLM-6172 adding modal focus changes #838

Merged
merged 5 commits into from
Jun 11, 2024
Merged

HLM-6172 adding modal focus changes #838

merged 5 commits into from
Jun 11, 2024

Conversation

siddhant-nawale-egov
Copy link
Contributor

No description provided.

Copy link
Contributor

coderabbitai bot commented Jun 11, 2024

Walkthrough

Walkthrough

The recent updates encompass various enhancements and fixes across multiple components in the microplanning module. Key changes include improved modal management, updated styling for better user interface consistency, map zoom level adjustments, and refined date handling in campaign selection. Additionally, a new CSS version has been integrated, and the logic for object comparison in utility functions has been optimized.

Changes

Files/Paths Change Summaries
micro-ui/.../microplanning.scss Added sticky positioning for thead, adjusted th and td padding, and introduced new classes for popup styling.
micro-ui/.../Hypothesis.js, micro-ui/.../RuleEngine.js, micro-ui/.../Upload.js Updated modal management functions, added CSS class toggling, and reorganized JSX structure.
micro-ui/.../Mapping.js Adjusted initial and minimum map zoom levels, modified map dragging bounds, and updated button label text.
micro-ui/.../MicroplanDetails.js Adjusted margin and padding styles for components within the MicroplanDetails function.
micro-ui/.../Navigator.js Added a conditional CSS class to the ActionBar component based on the currentPage ID.
micro-ui/.../SelectCampaign.js Removed max property for startDate and dynamically set min for endDate based on the current date.
micro-ui/.../employee/index.js Removed import of UIConfiguration and modified the state object in the SETINITDATA action.
micro-ui/.../utils/index.js Updated computeDifferences function to use a new areObjectsEqual function for object comparison.
micro-ui/web/public/index.html Updated @egovernments/digit-ui-css version from 1.0.37-microplan to 1.0.38-microplan.
node_modules/.yarn-integrity Introduced a JSON configuration file with system parameters, module folders, flags, and other metadata related to Yarn package management.

Poem

In the realm of code, where changes dance,
Modals now toggle with a graceful prance.
Maps zoom tighter, boundaries redefined,
Dates align, with the current time in mind.
Styles refined, for a user-friendly spree,
Celebrating updates, from a coding bunny, that's me! 🐰✨


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>.
    • 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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @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 as 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.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration 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

Outside diff range and nitpick comments (18)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (4)

104-106: Add a brief description for the areObjectsEqual function.

It's good practice to include a comment explaining what this function does, especially since it seems to be used for comparing objects based on specific properties.


Line range hint 155-155: Replace flatMap with flat to simplify the code.

- resourceMapping = resourceMapping.flatMap((inner) => inner);
+ resourceMapping = resourceMapping.flat();

According to Biome, using flat() directly is more appropriate here since the callback function is an identity function, which is unnecessary.

Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


Line range hint 169-171: Convert function expression to an arrow function for consistency.

- value[0].forEach(function (item, index) {
+ value[0].forEach((item, index) => {

Using arrow functions can make the code more concise and maintain the lexical context of this more predictably.

Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


Line range hint 206-206: Use flat instead of flatMap for array flattening.

- resourceMapping = resourceMapping.flatMap((inner) => inner);
+ resourceMapping = resourceMapping.flat();

This change is recommended by Biome to simplify the code by removing unnecessary callback in flatMap.

Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (3)

166-166: Ensure consistent use of quotes for class names.

Consider using consistent quote styles (either single or double) throughout your codebase for class names to maintain code cleanliness.


Line range hint 386-386: Avoid using array indices as keys in React lists.

Using indices as keys in React can lead to performance issues and bugs, especially if the order of items changes. It's better to use unique identifiers.


Line range hint 373-373: Specify button types explicitly to prevent unintended form submissions.

- <button className="delete-button invisible" onClick={() => deleteHandler(item)}>
+ <button type="button" className="delete-button invisible" onClick={() => deleteHandler(item)}>
- <button className="delete-button delete-button-help-locator" onClick={() => deleteHandler(item)}>
+ <button type="button" className="delete-button delete-button-help-locator" onClick={() => deleteHandler(item)}>

This change prevents buttons from submitting forms unintentionally, which is the default behavior when the type attribute is not specified.

Also applies to: 414-414

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (3)

Line range hint 494-494: Specify the button type explicitly to prevent unintended form submissions.

- <button className="add-button" onClick={() => addRulesHandler(setRules)} aria-label="Add Rules" role="button">
+ <button type="button" className="add-button" onClick={() => addRulesHandler(setRules)} aria-label="Add Rules" role="button">

This change ensures that the button does not inadvertently submit forms if nested within a form element.

Also applies to: 578-578


Line range hint 506-506: Avoid using array indices as keys in React components.

Using array indices as keys can lead to performance issues and bugs in React components when the order of items changes. It's better to use unique, stable identifiers.


Line range hint 829-838: Replace flatMap with flat for better performance and clarity.

- const finalData = filteredSchemas
-   ?.map((item) =>
-     Object.entries(item?.schema?.Properties || {}).reduce((acc, [key, value]) => {
-       if (value?.isRuleConfigureInputs) {
-         acc.push(key);
-       }
-       return acc;
-     }, [])
-   )
-   .flatMap((item) => item)
-   .filter((item) => !!item);
+ const finalData = filteredSchemas
+   ?.map((item) =>
+     Object.entries(item?.schema?.Properties || {}).reduce((acc, [key, value]) => {
+       if (value?.isRuleConfigureInputs) {
+         acc.push(key);
+       }
+       return acc;
+     }, [])
+   )
+   .flat()
+   .filter((item) => !!item);

This change optimizes the code by removing an unnecessary flatMap call, which is effectively just a flat operation in this context.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (8)

Line range hint 38-38: Replace hasOwnProperty with Object.hasOwn.

- if (obj.hasOwnProperty(key)) {
+ if (Object.hasOwn(obj, key)) {

This change is recommended to avoid issues that arise from the prototype chain. Object.hasOwn() is a safer alternative as it does not check the prototype chain.


Line range hint 266-266: Simplify the ternary operation.

Consider simplifying the ternary operation for better readability. If the intent is to ensure a boolean value, use Boolean(expression) or !!expression.


Line range hint 519-519: Ensure accessibility by pairing mouse events with keyboard events.

+ onKeyUp={yourKeyUpHandler}

Add keyboard event handlers (onKeyUp, onKeyDown, or onKeyPress) to elements that have mouse event handlers to ensure accessibility for users who rely on keyboard navigation.

Also applies to: 572-572, 573-573, 711-718, 826-826, 836-842


Line range hint 524-524: Remove tabIndex from non-interactive elements.

- tabIndex={0}

Non-interactive elements should not be focusable. Removing tabIndex will improve accessibility by preventing non-interactive elements from receiving keyboard focus.


Line range hint 562-562: Avoid assignments within expressions.

Consider refactoring this line to separate the assignment from the expression to enhance clarity and maintainability.


Line range hint 917-917: Use optional chaining for safer property access.

- if (someObject && someObject.someProperty)
+ if (someObject?.someProperty)

Using optional chaining (?.) simplifies the checks for nested properties and ensures that your code does not throw errors if an intermediate property is null or undefined.

Also applies to: 926-926, 927-927, 956-957, 1058-1058, 1060-1060


Line range hint 1057-1057: Declare variables at the function root.

Move the declaration of variables to the top of the function to avoid potential hoisting issues and enhance readability.

Also applies to: 1068-1068


Line range hint 1068-1068: Wrap declarations in blocks within switch cases.

case XYZ:
+ {
  let someVariable;
  // code
+ }

To prevent variables in a switch case from being accessible in other cases, wrap the declarations in a block. This encapsulates the scope to the specific case block.

Also applies to: 1072-1082

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 571db1d and e59be5c.

Files ignored due to path filters (4)
  • micro-ui/web/micro-ui-internals/packages/css/package.json is excluded by !**/*.json
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/package.json is excluded by !**/*.json
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/UIConfiguration.json is excluded by !**/*.json
  • yarn.lock is excluded by !**/*.lock, !**/*.lock
Files selected for processing (12)
  • micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (3 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (5 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js (1 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (6 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (8 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/index.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (2 hunks)
  • micro-ui/web/public/index.html (1 hunks)
  • node_modules/.yarn-integrity (1 hunks)
Files not reviewed due to errors (1)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (Error: Server error. Please try again later.)
Files skipped from review due to trivial changes (3)
  • micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss
  • micro-ui/web/public/index.html
  • node_modules/.yarn-integrity
Additional context used
Path-based instructions (9)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/index.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (1)

Pattern **/*.js: check

Learnings (2)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (1)
User: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js:55-57
Timestamp: 2024-05-27T04:08:38.193Z
Learning: The logic for the data change save check in the `RuleEngine.js` file has been shifted elsewhere for more global checks.
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (2)
User: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#204
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:468-481
Timestamp: 2024-03-13T07:33:45.211Z
Learning: The error message in the `checkForErrorInUploadedFile` function within `Upload.js` is being localized and improved for better user experience, as clarified by the user.
User: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#204
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:0-0
Timestamp: 2024-03-13T05:11:36.400Z
Learning: A more detailed message for file parsing errors in the `Upload.js` file is displayed elsewhere in the code, as clarified by the user.
Biome
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js

[error] 212-220: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)

Unsafe fix: Use a SelfClosingElement instead

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js

[error] 115-115: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 144-148: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 229-229: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 277-277: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.


[error] 279-279: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.


[error] 280-280: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 283-283: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 155-155: Avoid unnecessary callback in flatMap call. (lint/correctness/noFlatMapIdentity)

You can just use flat to flatten the array.
Safe fix: Replace unnecessary flatMap call to flat instead.


[error] 169-171: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 180-180: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 206-206: Avoid unnecessary callback in flatMap call. (lint/correctness/noFlatMapIdentity)

You can just use flat to flatten the array.
Safe fix: Replace unnecessary flatMap call to flat instead.


[error] 229-229: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 325-325: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 396-396: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js

[error] 49-52: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 54-57: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 69-69: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 81-81: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 391-399: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 496-496: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 502-502: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 373-373: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset


[error] 386-386: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 414-414: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset


[error] 574-574: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.


[error] 579-579: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js

[error] 54-57: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 59-62: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 154-154: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 166-166: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 187-187: Unsafe usage of optional chaining. (lint/correctness/noUnsafeOptionalChaining)

If it short-circuits with 'undefined' the evaluation will throw TypeError here:


[error] 505-512: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 695-695: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 716-716: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 829-838: Avoid unnecessary callback in flatMap call. (lint/correctness/noFlatMapIdentity)

You can just use flat to flatten the array.
Safe fix: Replace unnecessary flatMap call to flat instead.


[error] 869-874: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 494-494: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset


[error] 506-506: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 578-578: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js

[error] 38-38: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 266-266: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with


[error] 519-519: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 524-524: The HTML element div is non-interactive. Do not use tabIndex. (lint/a11y/noNoninteractiveTabindex)

Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.


[error] 562-562: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 572-572: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 573-573: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 711-718: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 826-826: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 836-842: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 917-917: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 926-926: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 927-927: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 956-957: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1057-1057: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)

The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.


[error] 1058-1058: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1060-1060: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1068-1068: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1068-1068: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)

The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.


[error] 1072-1082: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js

[error] 116-119: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 121-124: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 159-173: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 228-228: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 291-291: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 561-561: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 703-708: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1129-1129: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 1152-1152: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 1220-1223: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1231-1231: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)

Unsafe fix: Use a SelfClosingElement instead


[error] 1368-1370: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1388-1388: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 1398-1451: Promise executor functions should not be async. (lint/suspicious/noAsyncPromiseExecutor)


[error] 1465-1467: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1591-1593: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1703-1703: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 1812-1812: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 710-710: Reassigning a catch parameter is confusing. (lint/suspicious/noCatchAssign)

The catch parameter is declared here:

Use a local variable instead.


[error] 1150-1150: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

Additional comments not posted (3)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (1)

121-121: LGTM! Ensure that setting the minimum end date to the current date plus one day aligns with the business logic for campaign scheduling.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (1)

242-242: LGTM! Ensure that the increased margins do not disrupt the UI layout or overlap with other components.

Also applies to: 275-275

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js (1)

218-218: LGTM! Ensure that the addition of the popup-wrap-rest-unfocus class aligns with the intended styling adjustments for modal focus.

@nipunarora-eGov nipunarora-eGov merged commit f69332b into microplan Jun 11, 2024
2 checks passed
@nipunarora-eGov nipunarora-eGov deleted the HLM-6172 branch June 11, 2024 10:31
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: 8

Outside diff range and nitpick comments (18)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (1)

Line range hint 212-220: Use self-closing tags for elements without children.

- <InboxSearchComposerV2 configs={configs} browserSession={SelectCampaignSession} additionalConfig={{ resultsTable: { onClickRow, } }}></InboxSearchComposerV2>
+ <InboxSearchComposerV2 configs={configs} browserSession={SelectCampaignSession} additionalConfig={{ resultsTable: { onClickRow, } }} />

This change adheres to JSX best practices for cleaner and more readable code.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (1)

Line range hint 277-283: Ensure proper use of keys in list rendering.

- <p key={index} className="microplan-naming-convention-instruction-list number">
+ <p key={`number-${index}`} className="microplan-naming-convention-instruction-list number">
- <p key={index} className="microplan-naming-convention-instruction-list text">
+ <p key={`text-${index}`} className="microplan-naming-convention-instruction-list text">

Using unique and descriptive keys helps React identify which items have changed, are added, or are removed, which can improve performance and prevent bugs.

Tools
Biome

[error] 277-277: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (4)

Line range hint 155-155: Replace unnecessary flatMap calls with flat for performance optimization.

- resourceMapping = resourceMapping.flatMap((inner) => inner);
+ resourceMapping = resourceMapping.flat();

Also applies to: 206-206

Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


Line range hint 169-171: Convert function expressions to arrow functions for consistency and to reduce function scoping complexity.

- const handleNavigationEvent = function(
+ const handleNavigationEvent = (
Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


Line range hint 180-180: Utilize optional chaining to safely access deeply nested properties.

- if (item && item.error && !item.filestoreId) return;
+ if (item?.error || !item?.filestoreId) return;

Also applies to: 229-229, 325-325

Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


Line range hint 396-396: Remove unnecessary else clause to simplify control flow.

- } else return rowData[inputIndex];
+ return rowData[inputIndex];
Tools
Biome

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (3)

27-27: Update the state variable name to reflect its purpose more clearly.

Consider renaming modal to modalState to align with the setter setModalState for clarity and consistency.


Line range hint 386-386: Avoid using array indices as keys in React lists.

Using array indices as keys in React can lead to performance issues and bugs during re-renders. It's better to use unique identifiers if available.


Line range hint 574-579: Replace isNaN with Number.isNaN for accurate checks.

- if (isNaN(value)) return;
+ if (Number.isNaN(Number(value))) return;

Using Number.isNaN provides a more accurate check as it does not coerce the type, which avoids false positives.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (3)

Line range hint 501-501: Ensure that all button elements have an explicit type attribute to prevent unexpected form submissions.

- <button className="delete-button" onClick={() => deleteHandler(item)} aria-label={t("DELETE")} role="button">
+ <button type="button" className="delete-button" onClick={() => deleteHandler(item)} aria-label={t("DELETE")} role="button">

- <button type="button" className="add-button" onClick={() => addRulesHandler(setRules)} aria-label="Add Rules" role="button">
+ <button type="button" className="add-button" onClick={() => addRulesHandler(setRules)} aria-label="Add Rules" role="button">

Also applies to: 585-585


Line range hint 513-513: Avoid using array indices as keys in React lists to prevent issues with component state and performance.

Consider using a unique identifier or a stable property of the items as keys instead of array indices.


Line range hint 836-845: Replace unnecessary flatMap call with flat to simplify the code.

- const finalData = filteredSchemas
-   ?.map((item) =>
-     Object.entries(item?.schema?.Properties || {}).reduce((acc, [key, value]) => {
-       if (value?.isRuleConfigureInputs) {
-         acc.push(key);
-       }
-       return acc;
-     }, [])
-   )
-   .flatMap((item) => item)
-   .filter((item) => !!item);
+ const finalData = filteredSchemas
+   ?.map((item) =>
+     Object.entries(item?.schema?.Properties || {}).reduce((acc, [key, value]) => {
+       if (value?.isRuleConfigureInputs) {
+         acc.push(key);
+       }
+       return acc;
+     }, [])
+   )
+   .flat()
+   .filter((item) => !!item);
micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (1)

2266-2272: Ensure the sticky header works across all supported browsers.

The use of position: sticky is a great way to keep the header visible, but it's worth verifying its behavior across all browsers, especially older versions or less common ones, to ensure consistent user experience.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (4)

Line range hint 38-38: Replace hasOwnProperty with Object.hasOwn.

- if (obj.hasOwnProperty(key)) {
+ if (Object.hasOwn(obj, key)) {

This change addresses the linting error and follows best practices for checking properties in objects, avoiding issues that can arise from the prototype chain.


Line range hint 519-519: Ensure keyboard accessibility for interactive elements.

- <div className="icon" onClick={() => setShowChoroplethOptions((previous) => !previous)}>
+ <div className="icon" onClick={() => setShowChoroplethOptions((previous) => !previous)} onKeyUp={() => setShowChoroplethOptions((previous) => !previous)} tabIndex={0}>

Interactive elements triggered by mouse clicks should also be accessible via keyboard events to support accessibility standards.

Also applies to: 572-573, 711-718, 826-826, 836-842


Line range hint 524-524: Remove inappropriate tabIndex from non-interactive elements.

- <div className="icon" tabIndex={0}>
+ <div className="icon">

Non-interactive elements should not be focusable. Removing tabIndex improves accessibility by preventing keyboard users from stopping focus on non-interactive elements.


Line range hint 1057-1057: Declare variables at the function root to avoid confusion.

+ let keys, values;
  switch (fileData?.fileType) {
    case EXCEL:
-     let columnList = Object.values(fileData?.data)?.[0]?.[0];
+     keys = Object.values(fileData?.data)?.[0]?.[0];
      let check = true;
      if (latLngColumns) {
        for (let colName of latLngColumns) {
-         check = check && columnList.includes(t(colName));
+         check = check && keys.includes(t(colName));
        }
      }
      dataAvailabilityCheck = check
        ? dataAvailabilityCheck === "partial"
          ? "partial"
          : dataAvailabilityCheck === "false"
          ? "partial"
          : "true"
        : dataAvailabilityCheck === "partial"
        ? "partial"
        : dataAvailabilityCheck === "false"
        ? "false"
        : "partial";
      let dataWithResources = Object.values(fileData?.data);
      if (resources && formulaConfiguration && hypothesisAssumptionsList && schema?.showResourcesInMappingSection) {
        dataWithResources = dataWithResources?.map((item) => {
          return Digit.Utils.microplan.addResourcesToFilteredDataToShow(
            item,
            resources,
            hypothesisAssumptionsList,
            formulaConfiguration,
            microplanData?.microplanPreview?.userEditedResources ? microplanData?.microplanPreview?.userEditedResources : [],
            t
          );
        });
      }

      let hasLocationData = false;
-     const convertedData = dataWithResources?.map((item) =>
+     values = dataWithResources?.map((item) =>
        item?.map((row, rowIndex) => {
          if (rowIndex === 0) {
            if (row.indexOf("features") === -1) {
              row.push("feature");
            }
            return row;
          }
          const latIndex = item?.[0].findIndex((cell) => cell === "lat");
          const lonIndex = item?.[0].findIndex((cell) => cell === "long");
          let properties = {};
          row.map((e, index) => {
            properties[item?.[0]?.[index]] = e;
          });
          if (latIndex !== -1 && lonIndex !== -1) {
            if (!hasLocationData) hasLocationData = true;
            const lat = row[latIndex];
            const lon = row[lonIndex];
            const feature = {
              type: "Feature",
              properties: properties,
              geometry: {
                type: "Point",
                coordinates: [lon, lat],
              },
            };
            row.push(feature);
          } else {
            row.push(null);
          }
          return row;
        })
      );

      if (hasLocationData) {
        if (Object.values(fileData?.data).length > 0 && filterProperty) {
          filterProperty?.forEach((item) => {
            Object.values(fileData?.data).forEach((data) => {
              let filterPropertyIndex = data?.[0].indexOf(item);
              if (filterPropertyIndex && filterPropertyIndex !== -1)
                data.slice(1).forEach((e) => {
                  return filterPropertiesCollector.add(e[filterPropertyIndex]);
                });
            });
          });
        }
      }
      // extract dada
      var { hierarchyLists, hierarchicalData } = processHierarchyAndData(hierarchy, convertedData);
      if (filterDataOrigin?.boundriesDataOrigin && filterDataOrigin?.boundriesDataOrigin.includes(fileData?.section))
        setBoundary = { ...setBoundary, [fileData.section]: { hierarchyLists, hierarchicalData } };
      else if (filterDataOrigin?.layerDataOrigin && filterDataOrigin?.layerDataOrigin.includes(fileData?.section))
        setFilter = { ...setFilter, [fileData.section]: { hierarchyLists, hierarchicalData } };
      break;
    case GEOJSON:
    case SHAPEFILE:
      dataAvailabilityCheck = dataAvailabilityCheck === "partial" ? "partial" : dataAvailabilityCheck === "false" ? "partial" : "true"; // Update data availability for GeoJSON or Shapefile
      // Extract keys from the first feature's properties
      var keys = Object.keys(fileData?.data.features[0].properties);
      keys.push("feature");

      // Extract corresponding values for each feature
      const values = fileData?.data?.features.map((feature) => {
        // list with features added to it
        const temp = keys.map((key) => {
          if (feature.properties[key] === "") {
            return null;
          }
          if (key === "feature") return feature;
          return feature.properties[key];
        });
        return temp;
      });

      if (fileData?.data?.features && filterProperty) {
        filterProperty?.forEach((item) => {
          if (Object.values(fileData?.data).length > 0) {
            fileData?.data?.features.forEach((e) => {
              if (e?.properties?.[item]) filterPropertiesCollector.add(e?.properties?.[item]);
            });
          }
        });
      }

      // Group keys and values into the desired format
      // let data = { [files[fileData]?.fileName]: [keys, ...values] };
      // Adding resource data
      let dataWithResources = [keys, ...values];
      if (resources && formulaConfiguration and hypothesisAssumptionsList) {
        dataWithResources = Digit.Utils.microplan.addResourcesToFilteredDataToShow(
          dataWithResources,
          resources,
          hypothesisAssumptionsList,
          formulaConfiguration,
          microplanData?.microplanPreview?.userEditedResources ? microplanData?.microplanPreview?.userEditedResources : [],
          t
        );
        let indexOfFeatureInDataWithResources = dataWithResources?.[0]?.indexOf("feature");
        dataWithResources = dataWithResources.map((item, index) => {
          if (index === 0) return item;
          let newProperties = {};
          keys.forEach((e) => {
            if (e === "feature") return;
            let index = dataWithResources?.[0]?.indexOf(e);
            newProperties[e] = item[index];
          });
          let newRow = _.cloneDeep(item);
          newRow[indexOfFeatureInDataWithResources] = { ...item[indexOfFeatureInDataWithResources], properties: newProperties };
          return newRow;
        });
      }

      // extract dada
      var { hierarchyLists, hierarchicalData } = processHierarchyAndData(hierarchy, [dataWithResources]);
      if (filterDataOrigin?.boundriesDataOrigin && filterDataOrigin?.boundriesDataOrigin.includes(fileData?.section))
        setBoundary = { ...setBoundary, [fileData.section]: { hierarchyLists, hierarchicalData } };
      else if (filterDataOrigin?.layerDataOrigin && filterDataOrigin?.layerDataOrigin.includes(fileData?.section))
        setFilter = { ...setFilter, [fileData.section]: { hierarchyLists, hierarchicalData } };
  }

This change ensures that variables are declared at the function root, which is a best practice in JavaScript to avoid hoisting issues.

Also applies to: 1068-1068

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (1)

Line range hint 1398-1451: Avoid using async functions as promise executors to prevent subtle bugs related to error handling and timing.

- new Promise(async (resolve, reject) => {
+ new Promise((resolve, reject) => {

Removing async from the promise executor and handling asynchronous operations inside the executor with .then() or await inside a separate async function can lead to clearer and more reliable code.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 571db1d and 870a862.

Files ignored due to path filters (4)
  • micro-ui/web/micro-ui-internals/packages/css/package.json is excluded by !**/*.json
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/package.json is excluded by !**/*.json
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/UIConfiguration.json is excluded by !**/*.json
  • yarn.lock is excluded by !**/*.lock, !**/*.lock
Files selected for processing (12)
  • micro-ui/web/micro-ui-internals/packages/css/src/components/microplanning.scss (3 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (5 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js (1 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (14 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (8 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/index.js (2 hunks)
  • micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (2 hunks)
  • micro-ui/web/public/index.html (1 hunks)
  • node_modules/.yarn-integrity (1 hunks)
Files skipped from review due to trivial changes (1)
  • node_modules/.yarn-integrity
Additional context used
Path-based instructions (9)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/index.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (1)

Pattern **/*.js: check

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (1)

Pattern **/*.js: check

Learnings (2)
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (1)
User: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#698
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js:55-57
Timestamp: 2024-05-27T04:08:38.193Z
Learning: The logic for the data change save check in the `RuleEngine.js` file has been shifted elsewhere for more global checks.
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (2)
User: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#204
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:0-0
Timestamp: 2024-03-13T05:11:36.400Z
Learning: A more detailed message for file parsing errors in the `Upload.js` file is displayed elsewhere in the code, as clarified by the user.
User: siddhant-nawale-egov
PR: egovernments/DIGIT-Frontend#204
File: micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/Upload.js:468-481
Timestamp: 2024-03-13T07:33:45.211Z
Learning: The error message in the `checkForErrorInUploadedFile` function within `Upload.js` is being localized and improved for better user experience, as clarified by the user.
Biome
micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js

[error] 212-220: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)

Unsafe fix: Use a SelfClosingElement instead

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js

[error] 115-115: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 144-148: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 229-229: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 277-277: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.


[error] 279-279: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.


[error] 280-280: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 283-283: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js

[error] 113-113: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 123-123: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 155-155: Avoid unnecessary callback in flatMap call. (lint/correctness/noFlatMapIdentity)

You can just use flat to flatten the array.
Safe fix: Replace unnecessary flatMap call to flat instead.


[error] 169-171: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 180-180: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 206-206: Avoid unnecessary callback in flatMap call. (lint/correctness/noFlatMapIdentity)

You can just use flat to flatten the array.
Safe fix: Replace unnecessary flatMap call to flat instead.


[error] 229-229: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 325-325: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 396-396: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js

[error] 49-52: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 54-57: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 69-69: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 81-81: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 391-399: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 496-496: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 502-502: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 373-373: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset


[error] 386-386: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 414-414: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset


[error] 574-574: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.


[error] 579-579: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js

[error] 54-57: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 59-62: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 153-153: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 165-165: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 512-519: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 702-702: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 723-723: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 836-845: Avoid unnecessary callback in flatMap call. (lint/correctness/noFlatMapIdentity)

You can just use flat to flatten the array.
Safe fix: Replace unnecessary flatMap call to flat instead.


[error] 876-881: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 501-501: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset


[error] 513-513: Avoid using the index of an array as key property in an element. (lint/suspicious/noArrayIndexKey)

This is the source of the key value.

The order of the items may change, and this also affects performances and component state.
Check the React documentation.


[error] 585-585: Provide an explicit type prop for the button element. (lint/a11y/useButtonType)

The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js

[error] 38-38: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 266-266: Unnecessary use of boolean literals in conditional expression. (lint/complexity/noUselessTernary)

Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with


[error] 519-519: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 524-524: The HTML element div is non-interactive. Do not use tabIndex. (lint/a11y/noNoninteractiveTabindex)

Adding non-interactive elements to the keyboard navigation flow can confuse users.
Unsafe fix: Remove the tabIndex attribute.


[error] 562-562: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 572-572: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 573-573: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 711-718: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 826-826: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 836-842: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 917-917: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 926-926: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 927-927: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 956-957: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1057-1057: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)

The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.


[error] 1058-1058: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1060-1060: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1068-1068: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1068-1068: This var should be declared at the root of the enclosing function. (lint/correctness/noInnerDeclarations)

The var is accessible in the whole body of the enclosing function.
To avoid confusion, it should be declared at the root of the enclosing function.


[error] 1072-1082: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js

[error] 116-119: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 121-124: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 159-173: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 228-228: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 291-291: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 561-561: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 703-708: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1129-1129: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 1152-1152: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event. (lint/a11y/useKeyWithClickEvents)

Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.


[error] 1220-1223: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 1231-1231: JSX elements without children should be marked as self-closing. In JSX, it is valid for any element to be self-closing. (lint/style/useSelfClosingElements)

Unsafe fix: Use a SelfClosingElement instead


[error] 1368-1370: This function expression can be turned into an arrow function. (lint/complexity/useArrowFunction)

Function expressions that don't use this can be turned into arrow functions.
Safe fix: Use an arrow function instead.


[error] 1388-1388: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 1398-1451: Promise executor functions should not be async. (lint/suspicious/noAsyncPromiseExecutor)


[error] 1465-1467: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1591-1593: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1703-1703: Do not access Object.prototype method 'hasOwnProperty' from target object. (lint/suspicious/noPrototypeBuiltins)

It's recommended using Object.hasOwn() instead of using Object.hasOwnProperty().
See MDN web docs for more details.


[error] 1812-1812: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 710-710: Reassigning a catch parameter is confusing. (lint/suspicious/noCatchAssign)

The catch parameter is declared here:

Use a local variable instead.


[error] 1150-1150: Missing key property for this element in iterable. (lint/correctness/useJsxKeyInIterable)

The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.

Additional comments not posted (15)
micro-ui/web/public/index.html (1)

24-24: Update to version 1.0.38-microplan of @egovernments/digit-ui-css.

This update ensures that the latest styles and fixes are applied to the project. It's crucial to verify that this change does not introduce any visual regressions.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/index.js (1)

82-82: Ensure correct propagation of state data.

The state data from the hcm-microplanning is being correctly spread into the local state. This is crucial for maintaining the integrity of the application state.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/pages/employee/SelectCampaign.js (1)

121-121: Adjustment to the min property for the endDate field.

Setting the minimum date to tomorrow is a logical change, ensuring that end dates are always future-dated. This prevents logical errors in the campaign scheduling.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/MicroplanDetails.js (1)

Line range hint 242-275: Styling adjustments for MicroplanDetails components.

The changes in margin and padding improve the visual layout and consistency of the UI components.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Nagivator.js (1)

218-218: The addition of the popup-wrap-rest-unfocus class to the ActionBar component enhances modal focus management. Ensure that this class is well-defined in your CSS and does not conflict with other styles.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/utils/index.js (1)

104-106: The function areObjectsEqual provides a simple comparison between two objects based on name and code properties. Ensure this meets all your equality check requirements across the application.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Hypothesis.js (2)

166-166: Ensure consistent use of CSS classes for UI consistency.

The addition of popup-wrap-rest-unfocus to sectionClass aligns with the PR's objective to manage modal focus changes effectively.


224-257: Review modal implementation for accessibility and correctness.

- <button type="button" className="add-button" onClick={() => addAssumptionsHandler(setAssumptions)}>
+ <button type="button" className="add-button" onClick={() => addAssumptionsHandler(setAssumptions)} type="button">

Ensure that all buttons have an explicit type attribute to prevent unexpected form submissions. This is particularly important for buttons within forms, as the default type for button elements in HTML is "submit".

Likely invalid or redundant comment.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/RuleEngine.js (1)

28-28: The renaming of setModal to setModalState is consistent with the PR's objective to enhance modal management. Ensure all references to the old setModal are updated across the project.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Mapping.js (3)

218-220: Updated zoom levels to enhance user experience.

The change from a zoom level of 2 to 3 will provide a closer initial view, which can be more user-friendly for detailed maps.


224-224: Adjusted map boundary constraints.

Expanding the vertical bounds slightly allows for a wider range of map navigation, which can improve user interaction without drifting too far from the relevant area.


546-546: Corrected button label to match updated functionality.

Changing the label from "CLEAR_FILTERS" to "CLEAR_FILTER" aligns with the functionality of clearing a single filter, enhancing clarity for the user.

micro-ui/web/micro-ui-internals/packages/modules/hcm-microplanning/src/components/Upload.js (3)

53-53: The renaming of setModal to setModalState is consistent with the changes described in the PR summary. This helps in distinguishing between setting the modal state and the new setModal function which handles modal toggling logic.


866-866: The addition of the popup-wrap-rest-unfocus class to the div element is consistent with the modal focus changes described in the PR summary. This ensures that the modal focus styling is applied correctly.


1333-1333: Consider using Object.hasOwn() instead of Object.prototype.hasOwnProperty.call() for checking properties on objects as it is safer and avoids issues that might arise from the prototype chain.

- if (jsonObject.hasOwnProperty(key)) {
+ if (Object.hasOwn(jsonObject, key)) {

This change prevents potential issues caused by modifications to the object prototype and aligns with modern JavaScript best practices.

Likely invalid or redundant comment.

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.

2 participants