Skip to content

fix(SDK-1286): make the cancel payroll dialog describe cancelling - #2652

Open
krisxcrash wants to merge 2 commits into
mainfrom
kw/fix/sdk-1286-cancel-payroll-confirmation-copy
Open

fix(SDK-1286): make the cancel payroll dialog describe cancelling#2652
krisxcrash wants to merge 2 commits into
mainfrom
kw/fix/sdk-1286-cancel-payroll-confirmation-copy

Conversation

@krisxcrash

@krisxcrash krisxcrash commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

Fixes SDK-1286 (Medium, Pay v2 / RRoP Test Fest).

⚠️ Needs a copy reviewer. The AC says "matches approved copy/Figma" and I could not see the Figma. Legacy gws-flows cancel_message_html is the parity target used here — please confirm the wording.

Root cause

Not a mis-wired key — the dialog reads the keys you would expect, and the gws-flows Dialog adapter renders children correctly. The copy itself was the problem.

Before: "Any changes you have made to this payroll will be saved."
Legacy gws-flows: "You may cancel this payroll and run it again later. Your changes will be saved. Run this payroll by {cutoff} to pay your employees on time."

The SDK had dropped the one clause that says what cancelling actually does. What was left reads as stray help text sitting next to a destructive confirmation, which is precisely the report.

Plus a real second bug

The deadline line interpolated dateFormatter.formatWithTime(...).time — only the time half of formatWithTime's { time, date }. So it rendered:

Run this payroll by 4:00 PM PDT to ensure your employees are paid on time.

No date at all. This is the "SDK cancel copy drops the run-by date" note in the ticket, confirmed.

Now rendered as "<time> on <date>", which is not an invented format: PayrollHistory already formats the same deadline that way, and alerts.directDepositDeadline in this very namespace uses {{time}} on {{date}}. PayrollOverview was the outlier.

Also applied to Payroll.PayrollHistory

PayrollHistory hosts a second cancel-payroll confirmation whose copy was byte-identical. Fixing only PayrollOverview would have left the two dialogs disagreeing, so both namespaces get the new wording. PayrollHistory already renders its deadline with the date, so it needed no code change.

Also fixed

confirmCancelCta had a stray leading space (" Yes, cancel payroll"), which the sibling PayrollList and PayrollHistory keys do not have.

Judgment call for review

I kept the netPayDebit > 0 gate on the deadline line. Legacy shows it unconditionally, but the deadline is only meaningful when money is actually moving, and the gate was not part of the report. Easy to drop if you disagree.

Testing

Two new cases in PayrollOverviewPresentation.test.tsx:

  • the dialog explains the payroll can be run again later
  • the deadline line contains both date and time

I verified the deadline test fails when the change is reverted to .time only.

Three existing call sites asserted the old copy and are updated: PayrollOverviewPresentation (x2) and PayrollConfiguration. The third was found by the full suite rather than by grep — worth knowing this copy has more consumers than it looks.

Regenerated derived files with the real codegen (i18n:generate, then endpoints:derive + docs:api:generate), since docs/reference is CI-checked by derive-check-src. The docs diff is exactly the five copy lines.

npx vitest run src/components/Payroll — 627 pass. npm run tsc, eslint, prettier — clean.

The dialog opened with "Any changes you have made to this payroll will be saved.",
which never says what cancelling actually does -- it read as stray help text next
to a destructive confirmation. Legacy gws-flows leads with the clause that matters
("You may cancel this payroll and run it again later. Your changes will be
saved."), so this restores that wording.

Also fixes the deadline line, which interpolated only the `time` half of
formatWithTime's { time, date } and so rendered "Run this payroll by 4:00 PM PDT"
with no date at all. Now rendered as "<time> on <date>", matching how
PayrollHistory already formats the same deadline and the existing
alerts.directDepositDeadline string.

Applied to Payroll.PayrollHistory too: it hosts a second cancel-payroll
confirmation whose copy was byte-identical, so fixing only one would have left the
two dialogs disagreeing. PayrollHistory already renders its deadline with the
date, so it needed no code change.

Dropped the stray leading space in confirmCancelCta, which the sibling
PayrollList and PayrollHistory keys do not have.

Updated the three call sites asserting the old copy (PayrollOverview x2,
PayrollConfiguration) and regenerated the derived i18n types and translations
reference.

Copy note for reviewers: legacy gws-flows cancel_message_html is the parity target
used here. This needs a copy reviewer to confirm against Figma.
@krisxcrash
krisxcrash marked this pull request as ready for review August 27, 2026 19:10
@krisxcrash
krisxcrash requested a review from a team as a code owner August 27, 2026 19:10

@serikjensen serikjensen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good but going to hold off approval for @aaronlee777 to sign off on the copy

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants