[$250] [Search v2.2] - List does not scroll down and "To" field is hidden when "Save search" button appears

[IuliiaHerets]

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.35-4
Reproducible in staging?: Y
Reproducible in production?: N
Email or phone of affected tester (no customers): applausetester+kh010901@applause.expensifail.com
Issue reported by: Applause Internal Team

Action Performed:

1. Go to staging.new.expensify.com
2. Go to Search.
3. Click Filters.
4. Scroll down to the bottom.
5. Click "To".
6. Select any user and save it.

Expected Result:

The list will auto scroll down to show "To" field when "Save search" button appears.

Actual Result:

The list does not auto scroll down and "To" field is hidden when "Save search" button appears.

Workaround:

Unknown

Platforms:

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6605317_1726482606930.20240916_182646.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021835635471984345939
• Upwork Job ID: 1835635471984345939
• Last Price Increase: 2024-09-23

Issue Owner

Current Issue Owner: @akinwale

[melvin-bot]

Triggered auto assignment to @VictoriaExpensify (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.

[melvin-bot]

Triggered auto assignment to @Gonals (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

[IuliiaHerets]

We think that this bug might be related to #wave-control

[github-actions]

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

1. Identify the pull request that introduced this issue and revert it.
2. Find someone who can quickly fix the issue.
3. Fix the issue yourself.

[lakchote]

Not a blocker, it's a minor UI bug. Putting the external label.

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~021835635471984345939

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)

[huult]

Edited by proposal-police: This proposal was edited at 2023-10-09T08:43:00Z.

Proposal

Please re-state the problem that we are trying to solve in this issue.

List does not scroll down and "To" field is hidden when "Save search" button appears

What is the root cause of that problem?

After applying the Save filter, the Save Search button becomes visible

App/src/pages/Search/AdvancedSearchFilters.tsx

Lines 310 to 317 in 19d037b

	{!SearchUtils.isCannedSearchQuery(queryJSON) && (
	<Button
	text={translate('search.saveSearch')}
	onPress={onSaveSearch}
	style={[styles.mh4, styles.mt4]}
	large
	/>
	)}

What changes do you think we should make in order to solve the problem?

We should calculate the current offset of the scroll and add the height of the Save Search button. Some thing like that:

// .src/pages/Search/AdvancedSearchFilters.tsx#L231
+    const {saveScrollOffset, getScrollOffset} = useContext(ScrollOffsetContext);
+    const route = useRoute();
+    const scrollViewRef = useRef<RNScrollView>(null);
+    const saveSearchRef = useRef<RefObject<View>>(null);

+    const onScroll = useCallback<NonNullable<ScrollViewProps['onScroll']>>((e) => {
+        if (e.nativeEvent.layoutMeasurement.height === 0) {
+            return;
+        }

+        saveScrollOffsetByName(route, e.nativeEvent.contentOffset.y);
+    }, []);

// .src/pages/Search/AdvancedSearchFilters.tsx#L289
+    function isScrollingPossible(scrollOffset: number | null): boolean {
+        return !!scrollOffset && !!scrollViewRef.current && !!saveSearchRef.current;
+    }

+    function measureAndScroll(scrollOffset: number, bufferHeight: number): void {
+        saveSearchRef.current?.measure((_, __, ___, height) => {
+            scrollViewRef.current?.scrollTo({
+                y: scrollOffset + height + bufferHeight,
+                animated: false,
+            });
+        });
+    }

// .src/pages/Search/AdvancedSearchFilters.tsx#L290
            <ScrollView
+                ref={scrollViewRef}
+                onScroll={onScroll}
+                onLayout={() => {
+                    const scrollOffset = getScrollOffsetByName(route);
+                    if (!scrollOffset || !isScrollingPossible(scrollOffset)) {
+                        return;
+                    }

+                    const BUFFER_HEIGHT = 16;
+                    measureAndScroll(scrollOffset, BUFFER_HEIGHT);
+                }}
+                scrollEventThrottle={16}
                 contentContainerStyle={[styles.flexGrow1, styles.justifyContentBetween]}
            >

// .src/pages/Search/AdvancedSearchFilters.tsx#L312
   <Button
                    text={translate('search.saveSearch')}
                    onPress={onSaveSearch}
                    style={[styles.mh4, styles.mt4]}
                    large
+                    ref={saveSearchRef}
                />

.src/components/ScrollOffsetContextProvider.tsx#L87
+ const saveScrollOffsetByName: ScrollOffsetContextValue['saveScrollOffsetByName'] = useCallback((name, scrollOffset) => {
+        scrollOffsetsRef.current[name] = scrollOffset;
+    }, []);

+    const getScrollOffsetByName: ScrollOffsetContextValue['getScrollOffsetByName'] = useCallback((name) => {
+        if (!scrollOffsetsRef.current) {
+            return;
+        }
+        return scrollOffsetsRef.current[name];
+    }, []);

+    const clearScrollOffsetByName: ScrollOffsetContextValue['clearScrollOffsetByName'] = useCallback((name) => {
+        if (!scrollOffsetsRef.current) {
+            return;
+        }
+        delete scrollOffsetsRef.current[name];
+    }, []);


// .src/components/ScrollOffsetContextProvider.tsx#L82
- if (!scrollOffsetkeysOfExistingScreens.includes(key)) {
+ if (
+                    !scrollOffsetkeysOfExistingScreens.includes(key) &&
+                    key !== ADVANCED_SEARCH_FILTER)
+                ) {
                    delete scrollOffsetsRef.current[key];
                }

// .src/components/Search/index.tsx#L132
+    useFocusEffect(
+        useCallback(() => {
+            clearScrollOffsetByName(ADVANCED_SEARCH_FILTER);
+        }, []),
+    );

Here is test branch

POC

Screen.Recording.2024-09-16.at.21.59.31.mov

[ChavdaSachin]

Proposal

Please re-state the problem that we are trying to solve in this issue.

• List does not scroll down and "To" field is hidden when "Save search" button appears

What is the root cause of that problem?

• scrollView offset is retained but after rendering saveButton it hides the last element from scrollView.

What changes do you think we should make in order to solve the problem?

• make saveButton always visible but disable the button if all filters are empty.

    <Button
        text={translate('search.saveSearch')}
        onPress={onSaveSearch}
        style={[styles.mh4, styles.mt4]}
        large
        isDisabled={SearchUtils.isCannedSearchQuery(queryJSON)}
    />

App/src/pages/Search/AdvancedSearchFilters.tsx

Lines 310 to 317 in 1f66401

	{!SearchUtils.isCannedSearchQuery(queryJSON) && (
	<Button
	text={translate('search.saveSearch')}
	onPress={onSaveSearch}
	style={[styles.mh4, styles.mt4]}
	large
	/>
	)}

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[drminh2807]

Proposal

Please re-state the problem that we are trying to solve in this issue.

List does not scroll down and "To" field is hidden when "Save search" button appears

What is the root cause of that problem?

The "Save search" button make scroll view layout change

What changes do you think we should make in order to solve the problem?

Auto-scroll to last visible position when layout changed

            <ScrollView
                ref={scrollViewRef}
                onLayout={(e) => {
                    const newPosition = positionRef.current - e.nativeEvent.layout.height;
                    if (newPosition < 0) {
                        return;
                    }
                    scrollViewRef.current?.scrollTo({
                        y: newPosition,
                        animated: false,
                    });
                }}
                onScroll={(e) => {
                    positionRef.current = e.nativeEvent.contentOffset.y + e.nativeEvent.layoutMeasurement.height;
                }}
                contentContainerStyle={[styles.flexGrow1, styles.justifyContentBetween]}
            >

What alternative solutions did you explore? (Optional)

N/A

[huult]

Proposal update

• Create function get/set/clear offset by name
• Add test branch

[VictoriaExpensify]

Hey @akinwale - can you please review these proposals and let us know what you think?

[ChavdaSachin]

Note

Here's the reason why I have recommended to always show the save button over the other approaches.(My Proposal)
I checked out @huult's solution and it again has wrong scrolling behavior for different case.

Check this video

https://github.com/user-attachments/assets/24e76771-4917-4157-a31d-8d58840ecfc2

Notice `Merchant` was scrolled out of view.

And @drminh2807's solution would also behave similar for this case.

So rather, always showing save button is fairly simple and effective solution.

cc. @akinwale

[huult]

Thank you for your note, @ChavdaSachin ,
My proposal directly addresses this issue. I will provide an update on your problem after testing during the PR phase. I will update it as follows:

+   const onScroll = useCallback<NonNullable<ScrollViewProps['onScroll']>>((event) => {
+    const { layoutMeasurement, contentOffset, contentSize } = event.nativeEvent;

+    if (layoutMeasurement.height === 0) return;

+    const isAtBottom = layoutMeasurement.height + contentOffset.y >= contentSize.height;

+    if (isAtBottom) {
+        saveScrollOffsetByName(ADVANCED_SEARCH_FILTER, contentOffset.y);
+    } else {
+      getScrollOffsetByName(ADVANCED_SEARCH_FILTER) &&  clearScrollOffsetByName(ADVANCED_SEARCH_FILTER);
+    }
+}, []);

POC

Screen.Recording.2024-09-19.at.13.44.58.mov

cc @akinwale

[ChavdaSachin]

@huult let me help you with the test cases here.

1. Edit first field and check scroll behavior.
2. Edit last field and check scroll behavior.
3. Make sure first field is hidden and then edit second field
4. Make sure last field is hidden and then edit second last field
5. Make sure first field is hidden and then edit last visible field
6. Make sure last field is hidden and then edit first visible field

Note

Perform all of the above test with and without saving.

[akinwale]

@ChavdaSachin Could you post a test video of your proposed solution?

[ChavdaSachin]

Make save button always visible but disable it until all search fields are empty

Screen.Recording.2024-09-19.at.11.04.59.AM.mov

[ChavdaSachin]

@akinwale my solution does not need the steps I mentioned above.

[ChavdaSachin]

@akinwale do you want me to make a video on steps I mentioned above

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[luacmartins]

This is the expected behavior and not an issue. Closing this issue