Move pre-selected day to top of list in the custom statement close date picker - #98419
Conversation
Codecov Report✅ Changes either increased or maintained existing code coverage, great job!
|
|
@linhvovan29546 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] |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Freeze the day that was selected when this list opened so the pre-selected day stays pinned to the top for the whole open/focus cycle, even as the live selection changes. | ||
| const initialSelectedDay = useInitialSelection(selectedDay, {resetOnFocus: true}); | ||
|
|
||
| const data = useMemo(() => { |
There was a problem hiding this comment.
❌ CLEAN-REACT-PATTERNS-0 (docs)
React Compiler is enabled in this codebase and automatically memoizes derived values inside components, so wrapping data in useMemo is redundant. The manual memoization adds a dependency array that must be maintained and can interfere with the compiler's own caching. This block was modified in this PR, so it should be updated to a plain expression. (Note: the companion useInitialSelection hook was written non-generically specifically so OXC's React Compiler can memoize it, which indicates this feature area is expected to compile with React Compiler. If a compliance check shows this file fails to compile, this rule does not apply.)
Drop the useMemo and compute data directly so the compiler can memoize it:
const allDays = CONST.DATE.MONTH_DAYS.map<CustomCloseDateListItem>((dayValue) => ({
value: dayValue,
text: dayValue.toString(),
keyForList: dayValue.toString(),
isSelected: dayValue === selectedDay,
}));
const orderedDays = moveInitialSelectionToTop(allDays, initialSelectedDay ? [initialSelectedDay.toString()] : []);
const data = debouncedSearchValue ? orderedDays.filter((day) => day?.text?.includes(debouncedSearchValue)) : orderedDays;Reviewed at: 6d6fb68 | Please rate this suggestion with 👍 or 👎 to help us improve! Reactions are used to monitor reviewer efficiency.
There was a problem hiding this comment.
@linhvovan29546 I didn't add any useMemo, should I refactor them ?
There was a problem hiding this comment.
I think we can if it’s that simple.
trjExpensify
left a comment
trjExpensify
left a comment
There was a problem hiding this comment.
Another one bites the dust.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppScreen.Recording.2026-09-03.at.13.58.40.movAndroid: mWeb ChromeScreen.Recording.2026-09-03.at.13.54.14.moviOS: HybridAppScreen.Recording.2026-09-03.at.14.00.30.moviOS: mWeb SafariScreen.Recording.2026-09-03.at.13.52.18.movMacOS: Chrome / SafariScreen.Recording.2026-09-03.at.13.50.51.mov |
|
@marufsharifi Can you please merge main again to resolve the test fail? |
|
🚧 robertjchen 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/robertjchen in version: 9.4.70-0 🚀
|
|
🤖 No help site changes are required for this PR. I did not create a docs PR. Why: This change only reorders items inside an existing picker. The pre-selected day now renders at the top of the 1–31 list instead of in its numeric position, stays pinned while searching, and no longer scrolls on mount. No setting, label, option, or workflow changes — a user configures the statement close date the same way before and after. What I checkedThe diff ( The docs. No article under The guidelines. Separate gap worth noting: the Statement close date setting itself is undocumented on the help site. That predates this PR and is out of scope for a docs update tied to it, but it may be worth filing on its own. @marufsharifi — there is no linked help site PR to review, since no article needed a change. If you think an article does need updating here, reply with |
|
🚀 Deployed to staging by https://github.com/robertjchen in version: 9.4.72-0 🚀
|
|
🤖 No help site changes are required for this PR — so no docs PR was created. Why: This change only reorders an existing list. The pre-selected day in the Custom day of month picker now renders at the top of the 1–31 list and stays pinned while you search or pick a different day. No setting, label, button, or capability was added, removed, or renamed, and the resulting statement close date behavior is unchanged. What I checkedPR diff: two files — Help site search: I searched all of
No article describes the day list or its order, so nothing goes stale from this PR. One pre-existing gap, unrelated to this PR: the Statement close date setting itself isn't documented anywhere in the company card articles. That's a separate content gap that existed before this change. Reply with @marufsharifi, no help site PR was created because this change doesn't alter documented behavior. Please confirm you agree, or point me at an article you think needs updating and I'll open the draft PR. |
6 participants
Explanation of Change
Part of the ongoing rollout to show the pre-selected item at the top when a selection list (re)opens. This applies that behavior to the company card statement close date → "Custom day of month" picker: the day that's currently set appears at the top of the list when the picker opens, stays pinned there while searching, and doesn't jump when you pick a different day.
Fixed Issues
$ #69184
PROPOSAL: #69184 (comment):
Tests
1.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
Screen.Recording.2026-08-29.at.2.52.00.PM.mov
Android: mWeb Chrome
Screen_Recording_20260829_145503_Chrome.mp4
iOS: Native
Screen.Recording.2026-08-29.at.3.00.24.PM.mov
iOS: mWeb Safari
Screen.Recording.2026-08-29.at.3.02.30.PM.mov
MacOS: Chrome / Safari
Screen.Recording.2026-08-29.at.2.43.34.PM.mov