Open lanitochka17 opened 2 weeks ago
Triggered auto assignment to @CortneyOfstad (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
@CortneyOfstad FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors
When writing a few letters to trigger a search, scrolling down to the middle of the results, selecting all the content on search input and deleting it, the chats list updated but the user is not redirected to the top. The list stays in the middle
When users delete all search text, the newFocusedIndex
is -1
Then we have the logic to prevent scrolling to -1
in
We want to prevent scrolling if index is -1 because at the beginning the search term is always -1 then we don't want to do any scroll (If not the list can be blink)
To fix that bug, we can update scrollToIndex
function to add new param isFirstRender
const scrollToIndex = useCallback(
(index: number, animated = true, isFirstRender = false) => {
const item = flattenedSections.allOptions.at(index);
if (!listRef.current || !item || (isFirstRender && index === -1)) {
return;
}
const itemIndex = index === -1 ? 0 : item.index ?? -1;
const sectionIndex = item.sectionIndex ?? -1;
listRef.current.scrollToLocation({sectionIndex, itemIndex, animated, viewOffset: variables.contentHeaderHeight});
},
// eslint-disable-next-line react-compiler/react-compiler, react-hooks/exhaustive-deps
[flattenedSections.allOptions],
);
Then update
to
scrollToIndex(focusedIndex, false, true);
We can consider to use const item = flattenedSections.allOptions[index]
instead of at
Didn't realize this was Android — so having Stevie do some testing for me!
Confirmed that this was reproducible so getting eyes on it! S/O to @slafortune for her help!
Job added to Upwork: https://www.upwork.com/jobs/~021846215092993792827
Triggered auto assignment to Contributor-plus team member for initial proposal review - @allgandalf (External
)
User not scrolled to top of chats list when scrolling down and deleting search
After deleting the search, scrolling to the top is not possible because newSelectedIndex
returns -1
when textInputValue === ''
(which occurs after the previous search is cleared).
https://github.com/Expensify/App/blob/7fca65bcd7d91dd2f77cfee0818d6bd3729639ce/src/components/SelectionList/BaseSelectionList.tsx#L568-L574
The function updateAndScrollToFocusedIndex
receives this -1
and passes it to scrollToIndex
. Inside scrollToIndex
, there's a condition that checks if the index is -1
. If true, it returns without performing the scroll, causing the issue
https://github.com/Expensify/App/blob/7fca65bcd7d91dd2f77cfee0818d6bd3729639ce/src/components/SelectionList/BaseSelectionList.tsx#L252-L254
To resolve this issue, we need to check if textInputValue
is empty after the search text is removed. If it is, we need to return to index === 0
to enable scrolling to the top.
// .src/components/SelectionList/BaseSelectionList.tsx#L568
- const newSelectedIndex =
- textInputValue === '' || (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length) ? -1 : 0;
+ const newSelectedIndex =
+ (textInputValue === '' && !prevTextInputValue) || (flattenedSections.selectedOptions.length !== prevSelectedOptionsLength && prevAllOptionsLength === flattenedSections.allOptions.length) ? -1 : 0;
I've tested the previous issue, and this condition still works well after the update
Triggered auto assignment to @stephanieelliott (Bug
), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.
Hey @stephanieelliott! I am heading OoO shortly (until 10/23) so reassigning to keep an eye on this! Thanks!
@allgandalf What do you think about my proposal? Thank you
@CortneyOfstad, @stephanieelliott, @allgandalf Huh... This is 4 days overdue. Who can take care of this?
@allgandalf Can you take a look at my proposal?
@daledah :
We can consider to use const item = flattenedSections.allOptions[index] instead of at
What do you mean by this in your alternative solution ?
@allgandalf Could you please review my proposal? My solution simplifies the process by updating the condition to check if the index is 0 after the text search is cleared.
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@allgandalf I mean we can use flattenedSections.allOptions[index]
here since array[-1] = undefined, but array.at(-1) returns the last element
Back from OoO — thanks @stephanieelliott for holding down the fort while I was OoO!
@allgandalf looks like there was a response to your clarifying question here — any feedback?
@CortneyOfstad @stephanieelliott @allgandalf this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
@CortneyOfstad, @stephanieelliott, @allgandalf Whoops! This issue is 2 days overdue. Let's get this updated quick!
Back from OoO — thanks @stephanieelliott for holding down the fort while I was OoO!
@allgandalf looks like there was a response to your clarifying question here — any feedback?
I will look at this on monday
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.47-1 Reproducible in staging?: Y Reproducible in production?: Y If this was caught during regression testing, add the test name, ID and link from TestRail: https://expensify.testrail.io/index.php?/tests/view/5068088&group_by=cases:section_id&group_order=asc&group_id=229066 Email or phone of affected tester (no customers): N/A Issue reported by: Applause - Internal Team
Action Performed:
Expected Result:
After deleting the content in search input, the chats list should be updated and the user should be navigated to the top of it
Actual Result:
When writing a few letters to trigger a search, scrolling down to the middle of the results, selecting all the content on search input and deleting it, the chats list updated but the user is not redirected to the top. The list stays in the middle
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
https://github.com/user-attachments/assets/bf95d924-c9f2-49e4-99eb-c0e986073338
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @allgandalf