-
Notifications
You must be signed in to change notification settings - Fork 10
refactor(app): unify modal management #495
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
Conversation
Deployment results
Logs #20744114670 |
frontend/app/components/redesign/components/ScriptReadyModal.tsx
Outdated
Show resolved
Hide resolved
| ) | ||
| } | ||
|
|
||
| export const useSaveResultModal = () => { |
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 feels like it shouldn't belong to this file.
sidvishnoi
left a comment
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.
will take care of this further next by creating a new save one-profile-at-a-time route API that uses the new profile type. |

Closes #479
Requested Changes
Reduce boilerplate, enforces consistent modal pattern and improves maintainability by centralizing modal control
DialogProviderusing react contextwidget,bannerandbanner-twoof modalsCustom hooks for modal logic:
useSaveResultModal- encapsulates save resultuseGrantResponseHandler- appropriate success/error modals after grant response flow after wallet ownership verificationNote to reviewer: the internal logic and state of individual modal types remain unchanged; only the orchestration and rendering approach has been unified.