fix: show edit icon for inline table cell editing#93907
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@nyomanjyotisa 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
Bot
requested review from
joekaufmanexpensify
and removed request for
a team
![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: a476b4672d
ℹ️ 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".
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| > | ||
| <PressableWithFeedback | ||
| accessibilityRole={CONST.ROLE.BUTTON} | ||
| accessibilityLabel="Edit cell" |
There was a problem hiding this comment.
❌ CONSISTENCY-7 (docs)
The accessibilityLabel="Edit cell" is a hardcoded user-facing string. Accessibility labels are surfaced to screen-reader users, so they must be localized via the translation method rather than hardcoded in English, otherwise the label cannot be translated for non-English users.
Add a translation entry (e.g. common.edit or a dedicated key) to src/languages/* and pull it through useLocalize:
const {translate} = useLocalize();
// ...
<PressableWithFeedback
accessibilityRole={CONST.ROLE.BUTTON}
accessibilityLabel={translate('common.edit')}
sentryLabel={CONST.SENTRY_LABEL.TABLE.EDITABLE_CELL}
// ...
>Reviewed at: a476b46 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
|
🚧 @dubielzyk-expensify has triggered a test Expensify/App build. You can view the workflow run here. |
This comment has been minimized.
This comment has been minimized.
joekaufmanexpensify
left a comment
joekaufmanexpensify
left a comment
There was a problem hiding this comment.
Excited for this!
|
I manage to get the stuck button when tapping right from an editing field into another editing field: CleanShot.2026-06-19.at.10.41.58.mp4 |
|
Also curious if design thinks we should go full height on the cell hover target. I think we have padding around the main content at the moment, but curious if we should change that. cc @Expensify/design for that. CleanShot.2026-06-19.at.10.42.48.mp4Also curious to hear what @JmillsExpensify and @trjExpensify thinks. It's feeling a lot better to me than what we had |
|
@joekaufmanexpensify could you please check this?#93907 (comment) Thanks. |
|
I could see going full height on cell hover target. Also fine though to merge as is too. I agree this is an improvement over what we currently have. |
Shouldn't clicking outside the field only blur the field? Like when you open a popover menu, any click outside the popover just closes the popover - it doesn't also perform another click action. Does that make sense? (NAB)
Yeah I wouldn't mind doing this, but also feel the same as Jason here:
|
|
Similar feelings on my end too, but I agree that full height makes more sense. |
|
@nyomanjyotisa, what is the next step? |
@x-dev90 could you address the feedback from the design team? #93907 (comment) and #93907 (comment) Also, regarding #93907 (comment), how should this behave on wide layouts that don't support hover? @Expensify/design MacOS-Chrome.mov |
I kinda think this feature should only work if you are on a device that supports hover. Otherwise just tap into the expense details and edit from there. |
|
Agree with that Shawn. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid-Native.mp4Android: mWeb ChromeAndroid-mWeb.Chrome.mp4iOS: HybridAppiOS-Native.mp4iOS: mWeb SafariiOS-mWeb.Safari.mp4MacOS: Chrome / SafariMacOS-Chrome.mp4 |
|
@x-dev90 Please update the test steps for clarity. Steps 6-7 seem off here
After step 5, the cell is already in edit mode, so clicking elsewhere in the same row (outside the pencil button) would blur/exit edit mode, not open the expense/report Also, please include recordings/screenshots for all platforms next time, even when we couldn't fully test on all of them |
|
@nyomanjyotisa I updated the test steps. Could you please take a look when you get a chance? |
|
@x-dev90 Could you address this comment? |
|
@nyomanjyotisa Thanks! Since we removed the |
|
@MonilBhavsar, Gentle bump. thanks. |
|
@x-dev90 do you mind "Pressing tab key case(accessibility) to QA and Test steps please |
|
@MonilBhavsar Thanks for pointing that out. I updated the edit button so it is not exposed as an accessibility element while hidden, but it remains keyboard-focusable for the Tab flow. I also updated the test steps. Could you please check again when you get a chance? |
|
Thanks! |
|
🚧 MonilBhavsar has triggered a test Expensify/App build. You can view the workflow run here. |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.4.29-0 🚀
|
|
🤖 Help site review complete — docs updates required. This PR changes how inline editing is triggered on the Spend table (desktop): instead of clicking anywhere in an editable cell, users now hover to reveal an edit icon (pencil) and click that to edit inline, while clicking elsewhere in the row opens the expense. The help site still documented the old "click the cell to edit" behavior, so I opened a draft PR to correct it. Draft help site PR: #95530 Articles updated:
@x-dev90, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to staging by https://github.com/MonilBhavsar in version: 9.4.31-0 🚀
|
|
🤖 Help site review — changes required This PR changes inline table editing behavior in the Spend table: editable cells now reveal a pencil edit icon on hover/focus, and clicking that icon (rather than the cell itself) enters edit mode. Clicking elsewhere in the cell/row now opens the expense/report. Three help articles described the old "click the cell to edit inline" behavior and needed updating. Draft help site PR: #95674 Files updated:
The @x-dev90, please review the linked help site PR and confirm it reflects the current behavior. Then mark the linked help site PR |
|
🚀 Deployed to production by https://github.com/grgia in version: 9.4.31-0 🚀
Bundle Size Analysis (Sentry): |
11 participants
Explanation of Change
Updates Spend table inline editable cells so hovering an editable cell shows a pencil edit button instead of the previous full-cell hover border. Editing is now triggered only from the edit button, allowing clicks on the rest of the row/cell to open the expense/report. For right-aligned fields such as amount, the edit button appears on the left side of the cell.
Fixed Issues
$#93242
PROPOSAL:#93242(comment)
Tests
Offline tests
Same as tests.
QA Steps
Same as tests.
// TODO: These must be filled out, or the issue title must include "[No QA]."
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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
N/A
Android: mWeb Chrome
N/A
iOS: Native
N/A
iOS: mWeb Safari
N/A
MacOS: Chrome / Safari
Screen.Recording.2026-06-18.at.11.01.12.AM.mov