Feat: add a new category within the category list#86177
Feat: add a new category within the category list#86177daledah wants to merge 18 commits intoExpensify:mainfrom
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.
|
melvin-bot
Bot
requested review from
ahmedGaber93
and removed request for
a team
|
@ahmedGaber93 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
JmillsExpensify
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: af61ad1801
ℹ️ 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".
|
@ahmedGaber93 please complete review 🙇♂️ |
|
@daledah I think the UX flow might be missing something. After adding a new category, it should be applied directly to the expense instead of returning to the list and requiring the user to select it again. CC @dylanexpensify for confirmation. |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppa.mp4Android: mWeb Chrome20260330212429779.mp4iOS: HybridAppScreen.Recording.2026-04-13.at.10.07.19.PM.moviOS: mWeb Safariiw.mp4MacOS: Chrome / Safari20260330180222407.mp4 |
|
@Expensify/design WDYT #86177 (comment)? |
Correct, that was what we had in mind! |
|
Let us know when that is fixed and we can run some test builds. |
|
@daledah could you please address this comment: #86177 (comment) and let me know when this is ready for review again? |
Screen.Recording.2026-03-30.at.16.08.25.mov@ahmedGaber93 i updated |
|
🚧 @shawnborton has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
I noticed that a sound plays when a category is created. What's the thinking there? Is that how category creation currently works? |
|
Otherwise this is testing well and looks good to me! |
ProposalWhat is the root cause of that problem?Race condition between Onyx subscriber notifications for When The The It disappears later when the What changes do you think we should make in order to solve the problem?In the Concretely: before calling What alternative solutions did you explore? (Optional)
Next Steps for Contributor+ team: Reply with |
|
@daledah Hope you can take a first look. You may also have permission to follow up with Melvin, if not, just tag me and I can ask him for anything else needed. Thanks! |
|
@ahmedGaber93 i fixed this bug android.mov |
|
@daledah, thanks for the updates. The Android issue is already resolved, but unfortunately, I encountered new issues during the last test. Native: On the add category screen, if I restarted the app (force close then reopen), it takes me back to the setting category screen instead of category picker screen. Screen.Recording.2026-04-13.at.11.07.44.PM.moviOS: blank page briefly appears after saving a new category. Screen.Recording.2026-04-13.at.10.07.19.PM.mov |
ios.movi fixed this bug
I can't reproduce this bug in IOS and Android android.movios.mov |
|
Let's get another C+ review soon, as we really need to wrap up this PR. It's a frequent source of customer frustration. |
|
Reviewing in a while |
|
@daledah Thanks for the updates. I am also not able to reproduce the first issue. The iOS issue is disappeared, but the new behavior now still not smoothed, using Currently, the user goes back to the category picker and then to the confirmation page. Expectedly, the user should go directly to the confirmation page. Screen.Recording.2026-04-15.at.8.26.34.PM.movScreen.Recording.2026-04-15.at.8.36.52.PM.movLet's also get @Expensify/design's feedback on whether this looks acceptable or needs to be fixed. |
|
Hmm. The double nav back is a bit jarring. I wonder if after you click save on the new category whether we should just go one level back to the category picker screen with the new category selected. I know users are likely to tap save from there, but if you've tapped add a new category I think it's less disorienting to just go one level back with it selected. Curious what the other designers think. |
|
I agree the double-back thing is weird, I think ideally the user would create the category and then go back to the expense overview screen where that newly created category is now the category that is set. I see your point about consistency but I feel like in almost all cases, if you make a new category from this flow, that means you want to use that category on your expense. |
|
@MelvinBot please investigate this issue. The root cause seems to be that we navigated twice, once in the |
Investigation: Double Navigation After Category CreationRoot cause confirmed: Two sequential navigations fire when saving a new category in the in-situ IOU flow, causing the category picker to flash briefly before the user reaches the confirmation page. Navigation #1: Navigation #2: The user sees the category picker between these two navigations. Recommended fix: In the
Then in // CreateCategoryPage.tsx, lines 72-77
if (pendingCategorySelection) {
completePendingCategorySelection(values.categoryName.trim());
// Navigate past the category picker directly to the previous screen
Navigation.goBack(backTo);
} else {
clearPendingCategorySelection();
Navigation.goBack(isQuickSettingsFlow ? ROUTES.SETTINGS_CATEGORIES_ROOT.getRoute(route.params.policyID, backTo) : undefined);
}// IOURequestStepCategory.tsx, lines 224-238
useEffect(() => {
if (pendingCategorySelection?.transactionID !== transactionID || !pendingCategorySelection?.categoryName) {
return;
}
const {categoryName} = pendingCategorySelection;
if (!policyCategories?.[categoryName]?.enabled) {
return;
}
clearPendingCategorySelection();
// Only set the category — don't navigate, since CreateCategoryPage already handled navigation
setMoneyRequestCategory(transactionID, categoryName, policy);
}, [pendingCategorySelection, policyCategories, transactionID, policy]);This way there's only one navigation (from Full flow trace
Next Steps: Reply with |
This is where I'm at. |
|
Agree with double backing thoughts here. @daledah nudge |
|
Agree with Shawn's thoughts. I don't think we can just drop them on the category select screen because that screen has no save/confirm button. So we'd either drop them there with the new category selected and they'd have to hit back (weird/unintuitive) or we'd drop them without it selected and they'd have to select it (inefficient/frustrating because like Shawn said, if they're creating this category it's most likely because they want to use it now). So I think we need to figure out how to go from the create category screen to the confirmation screen. |
|
@daledah @ahmedGaber93 what's the hurdle/lift technically to have it just go right back to expense details/confirmation screen w/o having that awkward bounce step in the category selector? |
@dylanexpensify Everything worked on all platforms except iOS. It seems we also have some issues with double navigation on iOS. I'll try to find another solution later this week. |
|
@mountiny do we need any navigation experts to weigh in? |
|
yeah we can tag team here, though @daledah can you summarize what the issue is so its easier for @adamgrzybowski @WojtekBoman @sumo-slonik to weigh in? |
This is like an issue on iOS native where firing two sequential navigations can cause a brief screen flash (either a white screen or just a flicker). More root cause we can check here |
|
I’ll take a look at it as soon as I fix the bugs with the tab navigator that came up in the last code review round. |
|
thank you @sumo-slonik!! |
|
@sumo-slonik mind giving us a rough ETA when we might be able to look at this one? |
|
We are investigating this with @sumo-slonik, we will try to respond by the end of the day tomorrow! |
11 participants
Explanation of Change
Fixed Issues
$ #85681
PROPOSAL: #85681 (comment)
Tests
Offline tests
Same as tests
QA Steps
Same as tests
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, 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.ScrollViewcomponent to make it scrollable when more elements are added to the page.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.mov
Android: mWeb Chrome
a-mweb.mov
iOS: Native
ios.mov
iOS: mWeb Safari
ios-mweb.mov
MacOS: Chrome / Safari
web.mov