Closed IuliiaHerets closed 1 month ago
melvin-bot[bot]
commented
1 month ago 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[bot]
commented
1 month ago Triggered auto assignment to @Gonals (DeployBlockerCash), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.
IuliiaHerets
commented
1 month ago We think that this bug might be related to #wave-control
github-actions[bot]
commented
1 month ago :wave: 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:
lakchote
commented
1 month ago Not a blocker, it's a minor UI bug. Putting the external label.
melvin-bot[bot]
commented
1 month ago Job added to Upwork: https://www.upwork.com/jobs/~021835635471984345939
melvin-bot[bot]
commented
1 month ago Triggered auto assignment to Contributor-plus team member for initial proposal review - @akinwale (External)
huult
commented
1 month ago Edited by proposal-police: This proposal was edited at 2023-10-09T08:43:00Z.
List does not scroll down and "To" field is hidden when "Save search" button appears
After applying the Save filter, the Save Search button becomes visible
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
ChavdaSachin
commented
1 month ago saveButton it hides the last element from scrollView.
<Button
text={translate('search.saveSearch')}
onPress={onSaveSearch}
style={[styles.mh4, styles.mt4]}
large
isDisabled={SearchUtils.isCannedSearchQuery(queryJSON)}
/>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
commented
1 month ago List does not scroll down and "To" field is hidden when "Save search" button appears
The "Save search" button make scroll view layout change
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]}
>N/A
huult
commented
1 month ago 
VictoriaExpensify
commented
1 month ago Hey @akinwale - can you please review these proposals and let us know what you think?
ChavdaSachin
commented
1 month ago [!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.
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
commented
1 month ago 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);
+ }
+}, []);cc @akinwale
ChavdaSachin
commented
1 month ago @huult let me help you with the test cases here.
[!NOTE] Perform all of the above test with and without saving.
akinwale
commented
1 month ago @ChavdaSachin Could you post a test video of your proposed solution?
ChavdaSachin
commented
1 month ago Make save button always visible but disable it until all search fields are empty
https://github.com/user-attachments/assets/21409cbc-7967-4247-9f2c-70242e09692d
ChavdaSachin
commented
1 month ago @akinwale my solution does not need the steps I mentioned above.
ChavdaSachin
commented
1 month ago @akinwale do you want me to make a video on steps I mentioned above
melvin-bot[bot]
commented
1 month ago 📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
luacmartins
commented
1 month ago This is the expected behavior and not an issue. Closing this issue
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:
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:
Screenshots/Videos
https://github.com/user-attachments/assets/506dde1a-c497-4934-8e02-e34fb4eaac13
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @akinwale