Gate NetSuite invoice-item preference selection behind a Save button (a11y On Input, PR 13) - #99239
Gate NetSuite invoice-item preference selection behind a Save button (a11y On Input, PR 13)#99239MelvinBot wants to merge 1 commit into
Conversation
…(a11y On Input, PR 13) Co-authored-by: truph01 <truph01@users.noreply.github.com>
melvin-bot
Bot
added
the
Melvin-Test-Web
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
Android testing — blocked before reaching the page under test
|
| Check | Result |
|---|---|
| Reach Export → Invoice item preference page | ❌ Blocked — NetSuite auth required |
| Tapping an option stages selection without navigating away | |
| Save button disabled/enabled by staged vs saved value | |
| Save commits: Create → back, Select → reveals sub-menu | |
| No JS console / red-box errors | ✅ None observed throughout the session |
What's needed to complete this: a test workspace with a real, authenticated NetSuite sandbox connection (or a mock connection path on the emulator). With those credentials in place, the same steps above would reach the page and the four staged-selection/Save checks could be run.
Note: I confirmed the code in DynamicNetSuiteInvoiceItemPreferenceSelectPage.tsx implements the described staging (draftPreference state, confirmButtonOptions.isDisabled = currentPreference === selectedValue, and savePreference navigating back only for CREATE), but I could not exercise it live on-device.
Web testing — blocked before reaching the page under test
|
| Check | Result |
|---|---|
| Reach Export → Invoice item preference page | ❌ Blocked — live NetSuite connection required |
| Clicking an option stages selection without navigating away | |
| Save button disabled/enabled by staged vs saved value | |
| Save commits: Create → back, Select → reveals sub-menu | |
| No JS console / red-box errors | ✅ None observed during the drive |
What's needed to complete this: a test workspace with a real, authenticated NetSuite sandbox connection. With those credentials configured, the same steps above would reach the page and the four staged-selection/Save checks could be run.
Note: the code in DynamicNetSuiteInvoiceItemPreferenceSelectPage.tsx implements the described staging (draftPreference state, confirmButtonOptions.isDisabled = currentPreference === selectedValue, and savePreference navigating back only for CREATE), but I could not exercise it live in the browser.
|
@MelvinBot Mark this PR as ready |
|
Done — this PR is now marked Ready for review ✅ |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e09afce524
ℹ️ 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".
| const selectedValue = Object.values(CONST.NETSUITE_INVOICE_ITEM_PREFERENCE).find((value) => value === config?.invoiceItemPreference) ?? CONST.NETSUITE_INVOICE_ITEM_PREFERENCE.CREATE; | ||
|
|
||
| const [draftPreference, setDraftPreference] = useState<ValueOf<typeof CONST.NETSUITE_INVOICE_ITEM_PREFERENCE>>(); | ||
| const currentPreference = draftPreference ?? selectedValue; |
There was a problem hiding this comment.
When the preference update fails after optimistically saving SELECT, updateNetSuiteOnyxData restores config.invoiceItemPreference to the old value, but draftPreference remains SELECT and always wins here. The checkmark and focus therefore continue to show SELECT while the title and invoice-item submenu follow the rolled-back CREATE value, leaving the page in a contradictory state; reconcile or clear the draft when the Onyx value rolls back. CLAUDE.mdL82-L87
Useful? React with 👍 / 👎.
|
Nothing to fix in the code — every automated code check (tests, ESLint, Oxfmt, typecheck, React Compiler, Storybook, unused-styles, knip, build, perf) is passing. The only two failing checks are review-process gates that require a human, not a code change:
Both resolve through the normal review flow, so there's no push for me to make here. |
|
Still nothing to fix here — the state is unchanged since my last comment. All automated code checks pass. The only two failing checks are review-process gates that need a human, not a code change:
There's no push I can make to turn these green. |
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Good for product.
|
The only failing check on this PR is PR Reviewer Checklist / checklist (run). Every other check is green (39 passing, 5 skipped). That check isn't a code or test failure — it's the manual reviewer checklist, which fails until a reviewer completes the checklist items in a review comment. There's nothing to fix in code and nothing to push, so I haven't made any changes. To clear it, a reviewer needs to copy in and complete the PR Reviewer Checklist. No action is needed on the branch itself. |
3 participants
Explanation of Change
This addresses a WCAG 3.2.2 "On Input" accessibility violation, part of the batched plan in issue #74834 (PR 13).
DynamicNetSuiteInvoiceItemPreferenceSelectPagecommitted the invoice-item preference and, whenCREATEwas chosen, navigated back the instant a row was tapped — the layout/context changed without the user explicitly confirming, which is disorienting for screen-reader and low-vision users.The page now stages the preference in local state and only commits it when the user presses the Save button (via
confirmButtonOptionsonSelectionList), matching the pattern already shipped in PRs 2–8. The Save button is disabled until the staged selection differs from the persisted value. The entangled reveal branch is preserved:updateNetSuiteInvoiceItemPreferenceruns (only when the value actually changed), andgoBack()is called only for theCREATEbranch —SELECTstays on the page.MenuItemWithTopDescription) reveal remains gated on the persistedconfig.invoiceItemPreference === SELECT, so the item picker only appears once theSELECTpreference is actually committed, keeping the item/preference config consistent.🤖 Generated by MelvinBot. Checks run locally:
lint-changed✅,typecheck-tsgo✅. React Compiler compliance check couldn't run in this environment (base-ref resolution); this file already uses grandfathered manual memoization, so CI will validate. Fullnpm test/ storybook smoke deferred to CI.Fixed Issues
$ #74834
PROPOSAL: #74834 (comment)
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari