-
Notifications
You must be signed in to change notification settings - Fork 54
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
Fix facilities dropdown #6030
Fix facilities dropdown #6030
Conversation
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.
Changes work for me in dev5! I don't have any suggestions on how to add test coverage around this though 🤔
fixing some e2e issues that were introduced with this fix 😅 |
8455edd
to
eb26331
Compare
@@ -362,7 +363,7 @@ const FacilityForm: React.FC<Props> = (props) => { | |||
className="margin-right-0 margin-top-05" | |||
type="submit" | |||
label="Save changes" | |||
disabled={isSubmitting || !isDirty} |
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.
this wasn't actually setting the save button to be disabled when we click the "Save changes" button. isSubmitted
would do that but there's an issue if you back out of the address confirmation modal -- it won't reactivate the "Save changes" button so I opted to remove this.
Ready for re-review! Thank you for your patience 😅 🙏 |
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.
LGTM!
Kudos, SonarCloud Quality Gate passed! |
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.
👍
FRONTEND PULL REQUEST
Related Issue
After the changes introduced in #5983, facilities in the dropdown were not being properly updated.
Changes Proposed
Fix the facilities dropdown in the header to update the facility names correctly when they are updated or added.
Additional Information
please let me know if there's a good way to add test coverage for this scenario 🤔
Testing
on dev5 for testing
Screenshots / Demos