Skip to content

Gate company card and Expensify Card feed selection behind a Save button (a11y On Input, PR 9) - #99240

Open
MelvinBot wants to merge 1 commit into
mainfrom
claude-a11y-card-feed-selectors-save
Open

Gate company card and Expensify Card feed selection behind a Save button (a11y On Input, PR 9)#99240
MelvinBot wants to merge 1 commit into
mainfrom
claude-a11y-card-feed-selectors-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 9). These two near-identical feed selectors committed the selected feed and navigated back the instant a row was tapped, so the layout/context changed without the user explicitly confirming — disorienting for screen-reader and low-vision users.

Both pages now stage the tapped main feed 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 2–8. The staged row is highlighted, and the Save button is disabled until the staged selection differs from the current feed. Per the plan, only the main feed selection is gated behind Save — the footer rows are left immediate because they kick off async link/verify flows:

  • WorkspaceCompanyCardFeedSelectorPageselectFeed stages; Save runs updateSelectedFeed + goBack. The "Add cards" and "from other workspaces" footer rows (selectOtherFeed, which links/verifies and may navigate to add/verify-work-email) stay immediate.
  • WorkspaceExpensifyCardFeedSelectorPageselectFeed stages; Save runs resetCardFlowState + updateSelectedExpensifyCardFeed + goBack. The "Issue card / Set up new program" and "from other workspaces" footer rows stay immediate. The empty-state ScrollView (no primary feeds) is unchanged.

🤖 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

…ton (a11y On Input, PR 9)

Co-authored-by: truph01 <truph01@users.noreply.github.com>
@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 Δ
...panyCards/WorkspaceCompanyCardFeedSelectorPage.tsx 0.00% <0.00%> (ø)
...ifyCard/WorkspaceExpensifyCardFeedSelectorPage.tsx 0.00% <0.00%> (ø)
... and 8 files with indirect coverage changes

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Android testing — blocked on account data

I could not live-verify the Save-gated feed selectors on Android. The core flow requires a workspace with at least one company-card feed and one Expensify Card feed, but the CI test account has zero of each, and creating a feed hits a 6-digit account-validation code (magic-code / MFA) screen that can't be bypassed in CI.

What I confirmed on device (standalone Android NewDot):

  • Company Cards and Expensify Card pages render their empty states cleanly (no crash), showing only the "Add cards" / "Issue new card" CTAs.

What I could not verify (needs ≥1 feed per surface):

  • ⛔ Save button present at the bottom of the feed selector list
  • ⛔ Tapping a feed row stages the selection (highlights) and stays on the page instead of navigating back
  • ⛔ Save is disabled when the staged feed equals the current one and enables after tapping a different feed
  • ⛔ Pressing Save commits the selection and navigates back

Because no feed exists, the SelectionList + Save-button branch of WorkspaceCompanyCardFeedSelectorPage / WorkspaceExpensifyCardFeedSelectorPage (the feeds.length > 0 path — the exact code this PR changes) never renders, so there was nothing to drive-test.

Screenshots (empty states only)

Expensify Card page empty state (no feeds) on Android

Expensify Card page empty state (no feeds) on Android

Company Cards page empty state (no feeds) on Android

Company Cards page empty state (no feeds) on Android
Code-level corroboration (read-only, not a live test)

The checked-out PR code implements exactly the described behavior:

  • selectFeed only calls setDraftFeed / setDraftFundID — no navigation on tap.
  • confirmButtonOptions uses text: translate('common.save') with isDisabled: !currentSelectedFeed || currentSelectedFeed === selectedFeedName (analogous for Expensify Card).
  • saveFeed() calls the update action and then goBack().

This confirms the logic is wired as intended, but it is not a substitute for the live UI verification above.

To get a full Android pass, this needs a test account/workspace pre-provisioned with ≥1 company-card feed and ≥1 Expensify Card feed so the feed selector actually renders. Happy to re-run once such an account is available.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

🤖 Web testing — blocked on account data

I could not live-verify the Save-gated feed selectors on Web. The change lives at Workspace → Company cards and Workspace → Expensify Card, but the CI test account has no workspace at all (the Workspaces tab shows "No workspaces yet"), so neither feed-selector page is reachable.

Even creating a fresh workspace wouldn't help: a new workspace has zero company-card and zero Expensify Card feeds connected (that needs a real bank/card connection), so there'd still be no feed rows to stage/select between.

What I could not verify (needs a workspace with ≥1 feed per surface):

  • ⛔ Save button present at the bottom of the feed selector list
  • ⛔ Clicking a feed row stages the selection (highlights) and stays on the page instead of navigating back
  • ⛔ Save is disabled when the staged feed equals the current one and enables after clicking a different feed
  • ⛔ Pressing Save commits the selection and navigates back
Screenshot

Workspaces page showing "No workspaces yet" on Web

Workspaces page showing 'No workspaces yet' on Web
Code-level corroboration (read-only, not a live test)

The PR code implements exactly the described behavior on both pages:

  • selectFeed only stages (setDraftFeed / setDraftFundID) — no navigation on click.
  • confirmButtonOptions uses text: translate('common.save') with isDisabled: !currentSelectedFeed || currentSelectedFeed === selectedFeedName (analogous for Expensify Card).
  • saveFeed() calls the update action, then goBack().

This confirms the logic is wired as intended, but it is not a substitute for the live UI verification above.

To get a full Web pass, this needs a test account/session pre-provisioned with a workspace that already has ≥1 company-card feed and ≥1 Expensify Card feed connected so the feed selector actually renders. Happy to re-run once such an account is available.


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 heyjennahay 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 (out of draft).


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: 4d8f57d164

ℹ️ 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".

Comment on lines 197 to +199
const selectFeed = (feed: ExpensifyFeedListItem) => {
setDraftFundID(feed.value);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Apply the draft before starting card issuance

When multiple primary feeds exist, selecting feed B now only updates draftFundID, but the still-visible Issue card footer continues to derive issueCardFundID from lastSelectedExpensifyCardFeedID. If the user selects B and then taps Issue card without first pressing Save, handleAddCardPress writes the old feed A back to Onyx, and the issuance flow uses that old feed via useDefaultFundID, potentially issuing the card under the wrong program. Derive this footer action from currentSelectedFundID or disable it until the draft has been saved.

Useful? React with 👍 / 👎.

@MelvinBot

Copy link
Copy Markdown
Contributor Author

I looked into the two failing checks — neither is a code problem, so there's nothing for me to push a fix for.

Check Why it's red How it clears
checklist It's the PR Reviewer Checklist — the annotation is "No PR Reviewer Checklist was found". It fails until a reviewer completes it. A reviewer completes the reviewer checklist.
Check independent approval The PR doesn't yet have an independent reviewer approval. An eligible reviewer approves the PR.

Both are process gates that only a human reviewer can satisfy — they're on the standard ignore list precisely because they can't be resolved with a code change. Every actual code/build/test check (ESLint, Oxfmt, typecheck, React Compiler, Storybook, Bun/unit tests, perf, builds) is green ✅.

No commit was pushed.


view run

@MelvinBot

Copy link
Copy Markdown
Contributor Author

Same situation as my previous comment — the only two red checks are human-only process gates, not anything I can fix with code:

  • checklist = the PR Reviewer Checklist ("No PR Reviewer Checklist was found") — clears when a reviewer completes it.
  • Check independent approval — clears when an eligible reviewer approves.

Every actual code/build/test check is green ✅. There's no code failure here, so I haven't pushed anything. These will resolve on their own once a reviewer approves and fills out the checklist.


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