-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Navigation Editor: Move modal state to ManageLocations component #32078
Conversation
Size Change: -109 B (0%) Total Size: 1.62 MB
ℹ️ View Unchanged
|
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.
So I've got mixed feelings about this one.
On one hand you're completely right that as of now we have no use cases where we need to manage the modal state outside of the component.
That said, I suspect we may do in future as the Navigation Editor changes.
Overall, however, I'm inclined to approve this as there's little point in retaining complexity when there are no known use cases to support it.
Description
Moves modal state from
useNavigationEditor
toManageLocations
component and manage it locally.Fixes #31837.
How has this been tested?
Types of changes
Code Quality
Checklist:
*.native.js
files for terms that need renaming or removal).