HCMPRE-1766: App Configuration Integration With Data Type Meta Data And Localisation #2186
Conversation
📝 WalkthroughWalkthroughThis pull request reorganizes the campaign manager module by updating component import paths and registrations, while removing unused components. New localization functionalities are introduced through additional hooks, services, and context providers. Enhancements include modifications to the UI components to support popups, localization updates, and improved rendering logic. Several new components and custom hooks (e.g., for searching localization and managing missing keys) are added to streamline localization management within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as UI Component
participant Query as useSearchLocalisation Hook
participant Service as searchLocalisationService
participant API as API Service
UI->>Query: Trigger localization search
Query->>Service: Call searchLocalisationService with params
Service->>API: Request localization data
API-->>Service: Return data/error
Service-->>Query: Provide response
Query-->>UI: Deliver localization results
sequenceDiagram
participant User as User
participant Wrapper as AppConfigurationWrapper
participant Popup as Popup Component
User->>Wrapper: Click button to open popup
Wrapper->>Wrapper: Toggle showPopUp state to true
Wrapper->>Popup: Render popup
User->>Popup: Click submit/close button
Popup->>Wrapper: Trigger state update
Wrapper->>Wrapper: Toggle showPopUp state to false
Possibly related PRs
Poem
Tip 🌐 Web search-backed reviews and chat
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 25
🔭 Outside diff range comments (3)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js (1)
133-190: Remove commented code blocks.Large blocks of commented code should be removed to improve code maintainability. If this code needs to be referenced later, consider moving it to a separate documentation file or tracking it in version control history.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldScreenWrapper.js (1)
28-236: Consider decomposing the component for better maintainability.The component handles multiple responsibilities (tabs, steps, fields, headers). Consider breaking it down into smaller, focused components:
- TabManager
- StepManager
- FieldManager
- HeaderFieldManager
This would improve:
- Code organization
- Testing
- Reusability
- Maintenance
Would you like me to help create a plan for decomposing this component?
🧰 Tools
🪛 Biome (1.9.4)
[error] 36-36: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
[error] 46-46: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
[error] 47-47: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
[error] 48-48: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
[error] 105-105: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 143-143: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
[error] 113-113: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 115-134: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
[error] 138-172: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.js (1)
40-355: Consider extracting common patterns to reduce code duplication.The switch statement contains significant code duplication across cases, particularly in the LabelFieldPair and delete button implementation.
Consider extracting these common patterns into separate components:
- Create a reusable DeleteButton component
- Create a reusable FieldLabel component that handles the label, mandatory marker, and tooltip
- Create a reusable FieldWrapper component that handles the InfoCard and LabelFieldPair
This would significantly reduce code duplication and improve maintainability.
🧰 Tools
🪛 Biome (1.9.4)
[error] 80-95: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 136-152: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 185-201: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 233-249: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 281-297: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 328-344: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
[error] 102-102: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 159-159: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 208-208: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 256-256: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 353-353: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 74-74: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 121-121: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 179-179: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 227-227: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 275-275: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 322-322: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js(3 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/SidePanel.js(0 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js(2 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchLocalisationService.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useSearchLocalisation.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppConfigurationWrapper.js(6 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.js(8 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldScreenWrapper.js(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationTable.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationWrapper.js(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js(1 hunks)
💤 Files with no reviewable changes (1)
- health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/components/SidePanel.js
🧰 Additional context used
📓 Path-based instructions (11)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useSearchLocalisation.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchLocalisationService.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldScreenWrapper.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationWrapper.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationTable.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppConfigurationWrapper.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldScreenWrapper.js
[error] 115-134: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationWrapper.js
[error] 63-63: Unnecessary use of boolean literals in conditional expression.
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
(lint/complexity/noUselessTernary)
[error] 51-51: Use Number.POSITIVE_INFINITY instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.
(lint/style/useNumberNamespace)
[error] 52-52: Use Number.POSITIVE_INFINITY instead of the equivalent global.
ES2015 moved some globals into the Number namespace for consistency.
Safe fix: Use Number.POSITIVE_INFINITY instead.
(lint/style/useNumberNamespace)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js
[error] 13-13: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js
[error] 18-18: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 21-21: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 40-40: Unnecessary use of boolean literals in conditional expression.
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
(lint/complexity/noUselessTernary)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.js
[error] 74-74: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 102-102: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 121-121: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 159-159: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 179-179: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 208-208: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 227-227: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 256-256: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 275-275: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 322-322: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 353-353: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 379-386: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
Actions triggered using mouse events should have corresponding keyboard events to account for keyboard-only navigation.
(lint/a11y/useKeyWithClickEvents)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppConfigurationWrapper.js
[error] 270-270: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 271-271: Unsafe usage of optional chaining.
If it short-circuits with 'undefined' the evaluation will throw TypeError here:
(lint/correctness/noUnsafeOptionalChaining)
[error] 264-264: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 265-265: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 284-292: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
[error] 293-301: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.
(lint/correctness/useJsxKeyInIterable)
🔇 Additional comments (7)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js (1)
1-1: LGTM!The import statement correctly imports the required context hook.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js (1)
269-269: Handle browser compatibility for crypto.randomUUID().The
crypto.randomUUID()method might not be supported in all browsers.Consider using a fallback implementation:
- id: crypto.randomUUID(), + id: crypto.randomUUID?.() || Math.random().toString(36).substring(2),Run this script to check browser compatibility:
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldScreenWrapper.js (1)
137-172: LGTM! Localization props are properly implemented.The fields mapping correctly uses useCustomT for localizing various text fields.
🧰 Tools
🪛 Biome (1.9.4)
[error] 143-143: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
[error] 138-172: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.js (1)
376-376: Verify the necessity of componentRef.The component uses a ref but doesn't seem to utilize it. If it's not being used, consider removing it.
✅ Verification successful
Component ref usage verification
The
componentRefis defined and assigned to an element viaref={componentRef}, but no further usage (like accessingcomponentRef.current) is evident in the file. If there's no planned DOM manipulation or additional functionality that requires this ref, it appears safe to remove it.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any usage of componentRef in the codebase rg -l "componentRef" .Length of output: 159
Script:
#!/bin/bash # Search for any reference to componentRef.current in the file to see if the ref is actively used rg "componentRef\.current" ./health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.jsLength of output: 165
Script:
#!/bin/bash # Search for any JSX usage where componentRef is assigned as a ref to an element. rg "ref\s*=\s*{componentRef}" ./health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.jsLength of output: 286
Script:
#!/bin/bash # Using escaped curly braces to accurately search for JSX usage assigning componentRef as a ref attribute rg "ref\s*=\s*\\{componentRef\\}" ./health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppFieldComposer.jsLength of output: 195
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/useSearchLocalisation.js (1)
4-10: LGTM! Well-structured custom hook.The hook follows React Query best practices with proper query key construction and configuration handling.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/services/searchLocalisationService.js (1)
41-45: LGTM! Robust error handling.The error handling is well-implemented with proper extraction of error details and structured error response.
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/hooks/index.js (1)
22-22: LGTM! Clean integration of the new hook.The useSearchLocalisation hook is properly imported and integrated into the campaign object.
Also applies to: 52-52
...ls/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js
Outdated
Show resolved
Hide resolved
...ls/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js
Outdated
Show resolved
Hide resolved
...es/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js
Show resolved
Hide resolved
...es/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js
Outdated
Show resolved
Hide resolved
...es/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js
Show resolved
Hide resolved
...modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationWrapper.js
Show resolved
Hide resolved
...modules/campaign-manager/src/pages/employee/appConfigurationScreen/AppLocalisationWrapper.js
Show resolved
Hide resolved
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/Module.js
Show resolved
Hide resolved
...odules/campaign-manager/src/pages/employee/appConfigurationScreen/AppConfigurationWrapper.js
Show resolved
Hide resolved
...odules/campaign-manager/src/pages/employee/appConfigurationScreen/AppConfigurationWrapper.js
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js(5 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js (1)
Pattern **/*.js: check
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js (1)
Pattern **/*.js: check
🪛 Biome (1.9.4)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js
[error] 18-18: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 21-21: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 40-40: Unnecessary use of boolean literals in conditional expression.
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
(lint/complexity/noUselessTernary)
[error] 85-85: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
⏰ Context from checks skipped due to timeout of 10000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (actions)
🔇 Additional comments (5)
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/useCustomT.js (2)
1-3: LGTM! Hook follows React conventions.The import and hook declaration follow React best practices.
14-20: LGTM! Good handling of missing keys and fallbacks.The implementation properly handles:
- Finding localization entries
- Adding missing keys
- Providing fallbacks when needed
health/micro-ui/web/micro-ui-internals/packages/modules/campaign-manager/src/pages/employee/appConfigurationScreen/DrawerFieldComposer.js (3)
11-23: Remove unreachablebreakstatements or simplify the switch.The
returnstatements in this switch already exit the function, making the subsequent break statements at lines 18 and 21 unreachable. These lines can be safely removed. This mirrors a previous review comment about simplifying the entire switch to a single expression, so marking as a repeated suggestion.switch (panelItem?.label) { case "infoText": case "label": case "helpText": case "innerLabel": - return drawerState?.[panelItem?.label]; - break; default: - return false; - break; }🧰 Tools
🪛 Biome (1.9.4)
[error] 18-18: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
[error] 21-21: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
40-40: Simplify boolean conversion.Using a ternary expression to convert a value to a boolean is unnecessary. This change was pointed out previously, so it’s considered a repeated suggestion.
- isCheckedInitially={drawerState?.[panelItem.label] ? true : false} + isCheckedInitially={Boolean(drawerState?.[panelItem.label])}🧰 Tools
🪛 Biome (1.9.4)
[error] 40-40: Unnecessary use of boolean literals in conditional expression.
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(lint/complexity/noUselessTernary)
270-270: Ensure environment support forcrypto.randomUUID().
crypto.randomUUID()may not be supported in older browsers or certain Node versions. Confirm its compatibility in your target environment or provide a fallback if needed.✅ Verification successful
Compatibility of
crypto.randomUUID()is well-supported in modern environments.
Based on the latest web query results:
Browsers:
- Chrome: Supported since v92
- Firefox: Supported since v95
- Safari: Supported since v15
- Edge: Supported since v92
- Opera: Supported from v78
- Note: Requires HTTPS/secure context.
Node.js:
- Experimental support starting from v14.17
- Initial/stable implementations noted from v15.6
- Fully stable in v16.x and later
For projects targeting modern browsers and current Node.js LTS versions, using
crypto.randomUUID()is appropriate. If legacy support is a concern, consider adding a polyfill.🏁 Scripts executed
The following scripts were executed for the analysis:
Web query:
Which browsers and Node.js versions support `crypto.randomUUID()` natively?Length of output: 8196
| if (!code) { | ||
| console.warn("useCustomT: code parameter is required"); | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider using a proper logging solution.
While the parameter validation is good, using console.warn directly might not be ideal for production environments. Consider:
- Using a proper logging service/utility
- Making the warning message more descriptive
- console.warn("useCustomT: code parameter is required");
+ const message = `useCustomT: 'code' parameter is required but received ${typeof code}`;
+ Digit.Utils.Logger.warn(message);📝 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.
| if (!code) { | |
| console.warn("useCustomT: code parameter is required"); | |
| return ""; | |
| } | |
| if (!code) { | |
| const message = `useCustomT: 'code' parameter is required but received ${typeof code}`; | |
| Digit.Utils.Logger.warn(message); | |
| return ""; | |
| } |
| const { locState, addMissingKey } = useAppLocalisationContext(); | ||
| const currentLocale = Digit?.SessionStorage.get("initData")?.selectedLanguage || "en_IN"; | ||
| if (!Array.isArray(locState)) { | ||
| console.warn("useCustomT: locState is not an array"); | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Enhance error handling for context and locale.
The current implementation could be more robust:
- The context destructuring assumes values will exist
- Locale retrieval could use more defensive programming
- Similar to above, console.warn should be replaced
- const { locState, addMissingKey } = useAppLocalisationContext();
+ const context = useAppLocalisationContext();
+ if (!context) {
+ Digit.Utils.Logger.error('useCustomT: Localisation context is undefined');
+ return '';
+ }
+ const { locState, addMissingKey } = context;
- const currentLocale = Digit?.SessionStorage.get("initData")?.selectedLanguage || "en_IN";
+ const currentLocale = Digit?.SessionStorage?.get?.("initData")?.selectedLanguage || "en_IN";📝 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.
| const { locState, addMissingKey } = useAppLocalisationContext(); | |
| const currentLocale = Digit?.SessionStorage.get("initData")?.selectedLanguage || "en_IN"; | |
| if (!Array.isArray(locState)) { | |
| console.warn("useCustomT: locState is not an array"); | |
| return ""; | |
| } | |
| const context = useAppLocalisationContext(); | |
| if (!context) { | |
| Digit.Utils.Logger.error('useCustomT: Localisation context is undefined'); | |
| return ''; | |
| } | |
| const { locState, addMissingKey } = context; | |
| const currentLocale = Digit?.SessionStorage?.get?.("initData")?.selectedLanguage || "en_IN"; | |
| if (!Array.isArray(locState)) { | |
| console.warn("useCustomT: locState is not an array"); | |
| return ""; | |
| } |
| {state?.MASTER_DATA?.DrawerPanelConfig?.map((panelItem, index) => { | ||
| if (isFieldVisible(panelItem)) { | ||
| return ( | ||
| <div> | ||
| <RenderField | ||
| panelItem={panelItem} | ||
| drawerState={drawerState} | ||
| setDrawerState={setDrawerState} | ||
| state={state} | ||
| updateLocalization={updateLocalization} | ||
| /> | ||
| </div> | ||
| ); | ||
| } | ||
| })} |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add a key prop to the mapped elements.
When rendering an array of elements with .map(), each child should have a unique key to avoid React warnings and ensure stable element identity.
- <div>
+ <div key={panelItem?.label ?? index}>📝 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.
| {state?.MASTER_DATA?.DrawerPanelConfig?.map((panelItem, index) => { | |
| if (isFieldVisible(panelItem)) { | |
| return ( | |
| <div> | |
| <RenderField | |
| panelItem={panelItem} | |
| drawerState={drawerState} | |
| setDrawerState={setDrawerState} | |
| state={state} | |
| updateLocalization={updateLocalization} | |
| /> | |
| </div> | |
| ); | |
| } | |
| })} | |
| {state?.MASTER_DATA?.DrawerPanelConfig?.map((panelItem, index) => { | |
| if (isFieldVisible(panelItem)) { | |
| return ( | |
| <div key={panelItem?.label ?? index}> | |
| <RenderField | |
| panelItem={panelItem} | |
| drawerState={drawerState} | |
| setDrawerState={setDrawerState} | |
| state={state} | |
| updateLocalization={updateLocalization} | |
| /> | |
| </div> | |
| ); | |
| } | |
| })} |
| const isFieldVisible = (field) => { | ||
| // If visibilityEnabledFor is empty, the field is always visible | ||
| if (field?.visibilityEnabledFor?.length === 0) return true; | ||
| return field?.visibilityEnabledFor?.includes(drawerState?.type); // Check if current drawerState type matches | ||
| }; |
There was a problem hiding this comment.
💡 Codebase verification
Action: Ensure safe handling of visibilityEnabledFor property usage
The current search only shows its usage in one file, and there is no evidence elsewhere in the repository that guarantees it is always initialized to an array. Without additional guarantees (such as default values or explicit type checking), an undefined or non-array value could cause runtime issues.
Please ensure that the data structure providing visibilityEnabledFor is controlled to be an array. If not, consider providing a fallback (e.g., using field?.visibilityEnabledFor ?? []) or adding runtime type-checks.
🔗 Analysis chain
Verify that visibilityEnabledFor is always an array.
If visibilityEnabledFor is undefined or not an array, calling .includes() can yield unexpected results or undefined. Ensure it’s guaranteed to be an array or use a safe fallback.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Searching references for "visibilityEnabledFor" to confirm it's always an array type.
rg -A 5 'visibilityEnabledFor'Length of output: 1548
| default: | ||
| return null; | ||
| break; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Remove unreachable break after return.
In the default case, the return null; statement makes the subsequent break; unreachable. Removing it helps avoid confusion and keeps the code clean.
default:
return null;
- break;📝 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.
| default: | |
| return null; | |
| break; | |
| default: | |
| return null; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 85-85: This code is unreachable
... because this statement will return from the function beforehand
(lint/correctness/noUnreachable)
Choose the appropriate template for your PR:
Feature PR
Feature Request
JIRA ID
Module
Description
Related Issues
Bugfix PR
Bugfix Request
JIRA ID
Module
Description
Root Cause
Related Issues
Release PR
Summary by CodeRabbit