refactor(app): unify modal management #495
Conversation
Deployment results
Logs #20718838710 |
| try { | ||
| await toolActions.saveConfig(toolState.lastSaveAction) | ||
| showSaveResult(toolState.lastSaveAction) | ||
| } catch (err) { | ||
| const error = err as Error | ||
| // @ts-expect-error TODO: type error.cause properly | ||
| const fieldErrors = error.cause?.details?.errors?.fieldErrors | ||
| showSaveResult(toolState.lastSaveAction, { | ||
| message: error.message, | ||
| fieldErrors | ||
| }) | ||
| } |
There was a problem hiding this comment.
This part seems to be creeping in here. Why grant response need to care about saving? Saving requires grant success, but grant success shouldn't have to know about saving.
There was a problem hiding this comment.
Maybe use onGrantSuccess something prop.
frontend/app/components/redesign/components/ScriptReadyModal.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Many changes to this file seem like bug fixes, beyond this modal refactor work. Can you split them as separate PR (after or before this PR)?
There was a problem hiding this comment.
I'll do 1-2 smaller PRs before this one
There was a problem hiding this comment.
let me create yet another PR to pull the simplified DialogProvider changes
| ) | ||
| } | ||
|
|
||
| export const useSaveResultModal = () => { |
There was a problem hiding this comment.
This feels like it shouldn't belong to this file.
sidvishnoi
left a comment
sidvishnoi
left a comment
There was a problem hiding this comment.
I presently can't save config from banner-two route, but the modals work well.
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.