ReviewOS

stacks/ts-cloud

dashboard: consolidate the two confirm patterns into a useOpConfirm composable

#125
Closed glennmichael123 opened this 2 months ago ·
2 months ago

Confirmation is currently a per-page convention, which is why gaps keep appearing (see the missing-confirmation issue). With 34 pages this will keep drifting.

Current state

  • 13 pages use the shared partial: /var/www/reviewos-reviewos/releases/3e057f9/resources/views/layouts/app.stx:3025:322: include error: Error loading include file .../partials/op-confirm: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/.../partials/op-confirm.stx' ╭──────────────────────────────────────────────────────────────╮ │  ERROR  Include Error │ ╰──────────────────────────────────────────────────────────────╯ File: app.stx (line 3025, col 322) Message: Error loading include file .../partials/op-confirm: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/.../partials/op-confirm.stx' Context:  3023: 3024: > 3025:

    Confirmation is currently a per-page convention, which is why gaps keep appearing (see the missing-confirmation issue). With 34 pages this will keep drifting.

    Current state

    • 13 pages use the shared partial: /var/www/reviewos-reviewos/releases/3e057f9/resources/views/layouts/app.stx:3025:1538: include error: Error loading include file .../partials/op-confirm: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/.../partials/op-confirm.stx' ╭──────────────────────────────────────────────────────────────╮ │  ERROR  Include Error │ ╰──────────────────────────────────────────────────────────────╯ File: app.stx (line 3025, col 1538) Message: Error loading include file .../partials/op-confirm: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/.../partials/op-confirm.stx' Context:  3023: 3024: > 3025:

      Confirmation is currently a per-page convention, which is why gaps keep appearing (see the missing-confirmation issue). With 34 pages this will keep drifting.

      Current state

      • 13 pages use the shared partial: /var/www/reviewos-reviewos/releases/3e057f9/resources/views/layouts/app.stx:3047:37: include error: Error loading include file .../partials/op-confirm: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/.../partials/op-confirm.stx' ╭──────────────────────────────────────────────────────────────╮ │  ERROR  Include Error │ ╰──────────────────────────────────────────────────────────────╯ File: app.stx (line 3047, col 37) Message: Error loading include file .../partials/op-confirm: ENOENT: no such file or directory, open '/var/www/reviewos-reviewos/releases/3e057f9/resources/views/partials/.../partials/op-confirm.stx' Context:  3045: .reactions { display: flex; flex-wrap: wrap; gap: 6px; align-items: center; } 3046: > 3047: .reaction-form { display: inline-flex; margin: 0; } 3048: 3049: .reaction { — serverless.stx:131, serverless/{functions,deployments,queues,scheduler,assets,data}, server/{sites,backups,actions,services,workers,deployments}.
      • 4 pages hand-roll an equivalent bar: server/firewall.stx:123-128, server/team.stx, serverless/secrets.stx:104-109, serverless/alarms.stx:118-123.
      • The askOp / cancelOp / runOp / confirmTok / confirmVerb / canRun block is copy-pasted nearly verbatim in 11 files.

      Divergences between the two

      • Button label: the partial says "Run"; hand-rolled ones say "Remove"/"Delete". server/firewall.stx:126 says "Run" while its trigger says "Remove".
      • The partial's confirm button is a plain btn; hand-rolled use btn danger — so the shared bar renders a non-danger button even for backup:restore.
      • All four hand-rolled bars omit the <pre class="action-output">, discarding stdout/stderr from the API.
      • type="button" is missing on the confirm/cancel buttons in the partial and all four copies.

      Suggested direction

      Extract a useOpConfirm(endpoint) composable so a destructive call can't be made without routing through it, and make the partial's label/severity configurable so the hand-rolled copies can be deleted. Ideally forgetting the confirm becomes impossible rather than merely discouraged.

      Related cleanup: four different feedback mechanisms coexist (opOutput/opShown, message/messageKind, msg/msgOk, output/outputShown) — worth unifying on one useMessage().

      3026: 3027: 
      — serverless.stx:131, serverless/{functions,deployments,queues,scheduler,assets,data}, server/{sites,backups,actions,services,workers,deployments}.
    • 4 pages hand-roll an equivalent bar: server/firewall.stx:123-128, server/team.stx, serverless/secrets.stx:104-109, serverless/alarms.stx:118-123.
    • The askOp / cancelOp / runOp / confirmTok / confirmVerb / canRun block is copy-pasted nearly verbatim in 11 files.

    Divergences between the two

    • Button label: the partial says "Run"; hand-rolled ones say "Remove"/"Delete". server/firewall.stx:126 says "Run" while its trigger says "Remove".
    • The partial's confirm button is a plain btn; hand-rolled use btn danger — so the shared bar renders a non-danger button even for backup:restore.
    • All four hand-rolled bars omit the <pre class="action-output">, discarding stdout/stderr from the API.
    • type="button" is missing on the confirm/cancel buttons in the partial and all four copies.

    Suggested direction

    Extract a useOpConfirm(endpoint) composable so a destructive call can't be made without routing through it, and make the partial's label/severity configurable so the hand-rolled copies can be deleted. Ideally forgetting the confirm becomes impossible rather than merely discouraged.

    Related cleanup: four different feedback mechanisms coexist (opOutput/opShown, message/messageKind, msg/msgOk, output/outputShown) — worth unifying on one useMessage().

    3026: 3027: 
    — serverless.stx:131, serverless/{functions,deployments,queues,scheduler,assets,data}, server/{sites,backups,actions,services,workers,deployments}.
  • 4 pages hand-roll an equivalent bar: server/firewall.stx:123-128, server/team.stx, serverless/secrets.stx:104-109, serverless/alarms.stx:118-123.
  • The askOp / cancelOp / runOp / confirmTok / confirmVerb / canRun block is copy-pasted nearly verbatim in 11 files.

Divergences between the two

  • Button label: the partial says "Run"; hand-rolled ones say "Remove"/"Delete". server/firewall.stx:126 says "Run" while its trigger says "Remove".
  • The partial's confirm button is a plain btn; hand-rolled use btn danger — so the shared bar renders a non-danger button even for backup:restore.
  • All four hand-rolled bars omit the <pre class="action-output">, discarding stdout/stderr from the API.
  • type="button" is missing on the confirm/cancel buttons in the partial and all four copies.

Suggested direction

Extract a useOpConfirm(endpoint) composable so a destructive call can't be made without routing through it, and make the partial's label/severity configurable so the hand-rolled copies can be deleted. Ideally forgetting the confirm becomes impossible rather than merely discouraged.

Related cleanup: four different feedback mechanisms coexist (opOutput/opShown, message/messageKind, msg/msgOk, output/outputShown) — worth unifying on one useMessage().

8 days ago

Roadmap relationship: tracked by #133 under master roadmap #132.

8 days ago

Implemented and pushed to main in cbc4c25 (refactor(ui): centralize operation confirmation).

What changed:

  • added a reusable useOpConfirm stx composable for staging, exact-token validation, busy locking, cancellation, execution, errors, focus/scroll, and output
  • migrated every shared-partial consumer plus the firewall, team membership, secrets, alarms, and SSH-key hand-rolled flows
  • shared partial now supports computed danger treatment and action labels, explicit button types, accessible prompt naming, focus refs, and live output
  • destructive calls are routed through the composable while each page supplies only its API executor
  • added focused severity tests

Verification: composable tests pass and all 46 UI routes build successfully.

Sign in to comment on this issue.