fix(SDK-1286): make the cancel payroll dialog describe cancelling - #2652
Open
krisxcrash wants to merge 2 commits into
Open
fix(SDK-1286): make the cancel payroll dialog describe cancelling#2652krisxcrash wants to merge 2 commits into
krisxcrash wants to merge 2 commits into
Conversation
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
marked this pull request as ready for review
August 27, 2026 19:10
serikjensen
reviewed
Aug 27, 2026
serikjensen
left a comment
Member
There was a problem hiding this comment.
Code looks good but going to hold off approval for @aaronlee777 to sign off on the copy
serikjensen
approved these changes
Aug 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes SDK-1286 (Medium, Pay v2 / RRoP Test Fest).
Root cause
Not a mis-wired key — the dialog reads the keys you would expect, and the gws-flows
Dialogadapter renderschildrencorrectly. 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 thetimehalf offormatWithTime's{ time, date }. So it rendered: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:PayrollHistoryalready formats the same deadline that way, andalerts.directDepositDeadlinein this very namespace uses{{time}} on {{date}}. PayrollOverview was the outlier.Also applied to Payroll.PayrollHistory
PayrollHistoryhosts 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
confirmCancelCtahad a stray leading space (" Yes, cancel payroll"), which the siblingPayrollListandPayrollHistorykeys do not have.Judgment call for review
I kept the
netPayDebit > 0gate 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:I verified the deadline test fails when the change is reverted to
.timeonly.Three existing call sites asserted the old copy and are updated:
PayrollOverviewPresentation(x2) andPayrollConfiguration. 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, thenendpoints:derive+docs:api:generate), sincedocs/referenceis CI-checked byderive-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.