-
Notifications
You must be signed in to change notification settings - Fork 19
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
Disable facility editing #1833
base: console
Are you sure you want to change the base?
Disable facility editing #1833
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request introduce the Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range comments (1)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Line range hint
1-179
: Consider adding error boundary protectionSince this component handles API mutations and state management, consider wrapping it with an error boundary to gracefully handle runtime errors, especially during API calls or state updates.
Example implementation:
class AccessibilityPopUpErrorBoundary extends React.Component { state = { hasError: false }; static getDerivedStateFromError(error) { return { hasError: true }; } render() { if (this.state.hasError) { return <div>Error loading accessibility settings</div>; } return this.props.children; } } // Usage: <AccessibilityPopUpErrorBoundary> <AccessibilityPopUp {...props} /> </AccessibilityPopUpErrorBoundary>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js
(1 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js
(4 hunks)health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (1)
Pattern **/*.js
: check
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (1)
Pattern **/*.js
: check
🔇 Additional comments (4)
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/securityPopUp.js (1)
119-119
: LGTM! Radio buttons disabled state implementation looks good.
The disabled state is correctly implemented using the editingDisabled
prop.
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/accessbilityPopUP.js (2)
7-7
: LGTM: Clean prop addition with backward compatibility
The new editingDisabled
prop with default false
is well-implemented and maintains backward compatibility for existing usage.
118-118
: LGTM: Consistent dropdown disabling implementation
The disabled
prop is consistently applied to both dropdowns, providing a uniform editing restriction mechanism.
Also applies to: 130-130
health/micro-ui/web/micro-ui-internals/packages/modules/microplan/src/components/FacilityPopup.js (1)
437-437
: LGTM: Consistent implementation of editing restrictions
The addition of editingDisabled={true}
to both AccessibilityPopUp
and SecurityPopUp
components aligns with the PR objective to disable facility editing. The implementation is consistent across both popups, ensuring a uniform user experience.
Let's verify that the editingDisabled
prop is properly handled in the popup components:
Also applies to: 444-444
✅ Verification successful
The editingDisabled prop is properly implemented and used correctly
The verification confirms that both AccessibilityPopUp
and SecurityPopUp
components properly implement the editingDisabled
prop:
- Both components accept
editingDisabled
with a default value offalse
- The prop is correctly used to:
- Disable radio buttons in SecurityPopUp
- Disable dropdown selections in AccessibilityPopUp
- Disable the save buttons in both components
The implementation in FacilityPopup.js passing editingDisabled={true}
is consistent with the intended behavior to prevent editing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check implementation of editingDisabled prop in popup components
# Expected: Find the prop usage in both AccessibilityPopUp and SecurityPopUp components
# Search for the editingDisabled prop implementation
rg -l "editingDisabled.*prop|interface.*editingDisabled" --type js
# Check the components' implementation
ast-grep --pattern 'const $name = ({ $$$, editingDisabled, $$$ }) => {
$$$
}'
Length of output: 150
Script:
#!/bin/bash
# Let's try a broader search for the popup components and their props
# First, find the relevant popup component files
fd -e js -e jsx "Popup|PopUp"
# Then search for editingDisabled usage with more context
rg -A 5 -B 5 "editingDisabled" --type js
# Also search for the specific component names mentioned in FacilityPopup
rg -A 5 -B 5 "AccessibilityPopUp|SecurityPopUp" --type js
Length of output: 27986
Summary by CodeRabbit
New Features
editingDisabled
prop inAccessibilityPopUp
andSecurityPopUp
components to control editability.editingDisabled
state.Bug Fixes
editingDisabled
along with existing conditions for enabling/disabling.Documentation
AccessibilityPopUp
andSecurityPopUp
components have been updated to include the neweditingDisabled
prop.