Skip to content

Gate workspace picker selection behind a Save button (a11y On Input, PR 11) - #99236

Open
MelvinBot wants to merge 1 commit into
mainfrom
claude-a11y-workspace-pickers-save
Open

Gate workspace picker selection behind a Save button (a11y On Input, PR 11)#99236
MelvinBot wants to merge 1 commit into
mainfrom
claude-a11y-workspace-pickers-save

Conversation

@MelvinBot

Copy link
Copy Markdown
Contributor

Explanation of Change

This addresses a WCAG 3.2.2 "On Input" accessibility violation, part of the batched plan in issue #74834 (PR 11). Three workspace-picker pages committed the selection (and navigated away) the instant a row was tapped, so the layout/context changed without the user explicitly confirming — disorienting for screen-reader and low-vision users.

All three pages now stage the tapped workspace in local state and only commit it when the user presses the Save button (via confirmButtonOptions on SelectionList), matching the pattern already shipped in PRs 3–8. The staged row is highlighted via selectedPolicyIDs, and the Save button is disabled until the staged selection differs from the current value:

  • DomainGroupPreferredWorkspacePage — plain preferred-workspace write, committed on Save.
  • SetDefaultWorkspacePage — the areCategoriesEnabled branch (navigate onward vs. goBack) and the navigateTo guard are preserved by having Save call the existing selectPolicy.
  • DynamicReportChangeWorkspacePage — the billing RESTRICTED_ACTION early-return, the commuter-exclusion distance guard, and the per-report-type API calls (moveIOUReportToPolicy… / changeReportPolicyAndInviteSubmitter / changeReportPolicy) are all preserved by having Save call the existing selectPolicy.

🤖 Generated by MelvinBot. Checks run locally: lint-changed ✅, typecheck-tsgo ✅. React Compiler compliance check couldn't run in this environment (base-ref resolution); no manual memoization was added, so CI will validate. Full npm test / storybook smoke deferred to CI.

Fixed Issues

$ #74834
PROPOSAL: #74834 (comment)

Tests

  • Verify that no errors appear in the JS console

Offline tests

QA Steps

// TODO: These must be filled out, or the issue title must include "[No QA]."

  • Verify that no errors appear in the JS console

PR Author Checklist

  • I linked the correct issue in the ### Fixed Issues section above
  • I wrote clear testing steps that cover the changes made in this PR
    • I added steps for local testing in the Tests section
    • I added steps for the expected offline behavior in the Offline steps section
    • I added steps for Staging and/or Production testing in the QA steps section
    • I added steps to cover failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
    • I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
    • I tested this PR with a High Traffic account against the staging or production API to ensure there are no regressions (e.g. long loading states that impact usability).
  • I included screenshots or videos for tests on all platforms
  • I ran the tests on all platforms & verified they passed on:
    • Android: Native
    • Android: mWeb Chrome
    • iOS: Native
    • iOS: mWeb Safari
    • MacOS: Chrome / Safari
  • I verified there are no console errors (if there's a console error not related to the PR, report it or open an issue for it to be fixed)
  • I followed proper code patterns (see Reviewing the code)
    • I verified that comments were added to code that is not self explanatory
    • I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
    • I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
  • If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
  • I followed the guidelines as stated in the Review Guidelines
  • I tested other components that can be impacted by my changes (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar are working as expected)
  • If a new CSS style is added I verified that:
    • A similar style doesn't already exist
    • The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))
  • If new assets were added or existing ones were modified, I verified that:
    • The assets are optimized and compressed (for SVG files, run npm run compress-svg)
    • The assets load correctly across all supported platforms.
  • If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
  • If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
  • If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
  • If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
  • If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
    • I verified that all the inputs inside a form are aligned with each other.
    • I added Design label and/or tagged @Expensify/design so the design team can review the changes.
  • I added unit tests for any new feature or bug fix in this PR to help automatically prevent regressions in this user flow.
  • If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.

Screenshots/Videos

Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari

…PR 11)

Co-authored-by: truph01 <truph01@users.noreply.github.com>
@MelvinBot
MelvinBot requested a review from a team August 22, 2026 03:57
@melvin-bot melvin-bot Bot added Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web labels Aug 22, 2026
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ Changes either increased or maintained existing code coverage, great job!

Files with missing lines Coverage Δ
src/pages/DynamicReportChangeWorkspacePage.tsx 0.00% <0.00%> (ø)
src/pages/SetDefaultWorkspacePage.tsx 0.00% <0.00%> (ø)
...omain/Groups/DomainGroupPreferredWorkspacePage.tsx 0.00% <0.00%> (ø)
... and 7 files with indirect coverage changes

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Ran the testing steps on Web (dev NewDot). Result: ✅ pass — tapping a workspace row now stages the selection (highlights it and enables Save) instead of committing and navigating away; the change only commits when Save is pressed.

Change workspace on an expense report (DynamicReportChangeWorkspacePage) — verified live:

  1. Opened the picker from the report's More → Change workspace: the current workspace is checked and Save is dimmed/disabled.
  2. Tapped the other workspace row → the picker stayed open (no navigation), the row became checked/promoted to the top, and Save switched from dimmed to enabled.
  3. Pressed Save → the "You moved this report!" modal appeared, the picker closed, and the report header updated to the newly selected workspace. The commit happened only on Save.

Set default workspace (SetDefaultWorkspacePage) — not exercised live: its trigger requires a member of 2+ group workspaces with no resolved active policy, which this fresh test account doesn't naturally reach once an active policy is set. The code implements the identical staged pattern (draftPolicyID state, onSelectRow only sets the draft, Save disabled until the staged value differs).

Domain group preferred workspace (DomainGroupPreferredWorkspacePage) — not tested: it's a domain-admin-only page not reachable with a standard test account.

No error dialogs/toasts were observed during the flow.

Evidence

Change workspace picker open — current workspace checked, Save dimmed/disabled

Change workspace picker open, current workspace checked, Save dimmed/disabled

Other workspace row tapped — staged (checked, promoted to top), still on picker, Save now enabled

Other workspace row tapped and staged, still on picker screen, Save now enabled

After pressing Save — "You moved this report!" confirmation, picker closed

After pressing Save: You moved this report confirmation modal, picker closed

Report header now shows the report on the newly selected Workspace 1

Report header now shows the report attached to the newly selected Workspace 1


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Ran the testing steps on Android (standalone NewDot, emulator). Result: ✅ pass — tapping a workspace row now stages the selection (highlights it and enables Save) instead of committing and navigating away; the change only commits when Save is pressed.

Change workspace on an expense report (DynamicReportChangeWorkspacePage) — verified live:

  1. Opened the picker from the report's More → Change workspace: the current workspace is checked and Save is disabled.
  2. Tapped the other workspace row → the picker stayed open (no navigation), the row became checked, and Save switched from disabled to enabled.
  3. Pressed Save → the "You moved this report!" confirmation appeared, and after dismissing it the report header updated to the newly selected workspace. The commit happened only on Save.

The other two pages in this PR weren't exercised on Android: Set default workspace (SetDefaultWorkspacePage) isn't reachable in a fresh account's natural state (its trigger requires 2+ group workspaces with no resolved active policy), and Domain group preferred workspace (DomainGroupPreferredWorkspacePage) is domain-admin-only. Both implement the identical staged-selection pattern in code.

No JS console errors or error toasts observed during the flow.

Note: "Change workspace" only appears in the More menu while the report is still Open/Draft, so the flow was exercised against a draft expense.

Evidence

Change workspace picker open — current workspace checked, Save disabled

Change workspace picker open, Save disabled, current workspace checked

Other workspace row tapped — staged (checked), still on picker, Save now enabled

Workspace 1 row staged/checked, Save button enabled, still on picker

After pressing Save — "You moved this report!" confirmation

You moved this report confirmation dialog after pressing Save

Report now on the newly selected Workspace 1

Report screen showing From ... in ...'s Workspace 1 after commit


view run

@truph01

truph01 commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

@MelvinBot Mark this PR as ready

@MelvinBot
MelvinBot marked this pull request as ready for review August 22, 2026 05:10
@MelvinBot
MelvinBot requested a review from a team as a code owner August 22, 2026 05:10
@melvin-bot
melvin-bot Bot requested review from trjExpensify and truph01 and removed request for a team August 22, 2026 05:10
@melvin-bot

melvin-bot Bot commented Aug 22, 2026

Copy link
Copy Markdown

@truph01 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

@melvin-bot
melvin-bot Bot removed the request for review from a team August 22, 2026 05:10
@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Done — this PR is now marked Ready for review.


view run

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f75446aef0

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

currentUserLogin: session?.email,
shouldShowPendingDeletePolicy: false,
selectedPolicyIDs: report.policyID ? [report.policyID] : undefined,
selectedPolicyIDs: currentSelection ? [currentSelection] : undefined,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Keep draft workspace selections in place until Save

When a user selects a workspace that is not already first—particularly while searching—passing the changing currentSelection to useWorkspaceList triggers its default shouldSortSelectedToTop behavior, immediately moving that row to the top before Save. During an active search, SelectionList retains its numeric focused index, so focus can now point to a different workspace and the next Enter press selects that unintended row; the immediate reorder also preserves the input-driven context shift this change is meant to prevent. Set shouldSortSelectedToTop: false here and in the corresponding SetDefaultWorkspacePage call while retaining selectedPolicyIDs for highlighting.

Useful? React with 👍 / 👎.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

The only failing checks are non-code gates that require human action, not a code fix — there's nothing here I can push a fix for:

  • PR Reviewer Checklist / checklist — fails because the reviewer checklist hasn't been completed yet. It passes once a reviewer fills it out.
  • Check independent approval — fails because the PR doesn't yet have the required independent approval. It passes once a reviewer approves.

Every actual code/build check is green: test (jobs 1–8), typecheck, ESLint, Oxfmt, React Compiler Compliance, Storybook tests, Bun tests, Check for unused styles, knip, and all build jobs all passed.

So no fix is needed — these two checks will clear on their own once the PR gets a reviewer + independent approval.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

No change since my last analysis — the only two failing checks (PR Reviewer Checklist / checklist and Check independent approval) are human-action gates, not code failures, so there's no fix to push. Every code/build check is green. These clear once a reviewer completes the checklist and approves.


view run

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

Labels

Melvin-Test-Android Melvin-Test-Web Triggers Melvin to run the testing steps of the PR on web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants