[HOLD for payment 2024-09-21][$250] [Search v2.2] Keep status scroll position and update label sizes #47640

luacmartins · 2024-08-19T15:45:24Z

Coming from this PR, we need to address these comments:

Issue Owner

Current Issue Owner: @

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~021833233716363835239
Upwork Job ID: 1833233716363835239
Last Price Increase: 2024-09-09

Issue Owner

Current Issue Owner: @mallenexpensify

melvin-bot · 2024-08-19T15:45:33Z

Triggered auto assignment to @mallenexpensify (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 · 2024-08-19T15:45:33Z

Unable to auto-create job on Upwork. The BZ team member should create it manually for this issue.

melvin-bot · 2024-08-19T15:45:37Z

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

nkdengineer · 2024-08-19T16:36:14Z

Proposal

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

The status scroll position isn't saved
The label size of Expense dropdown button isn't correct

What is the root cause of that problem?

When we click on status option, we open a new page and then SearchStatusBar is mounted again which makes the scroll position reset.
The font size of Expense dropdown button isn't correct because it has the default font-size

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 78 in 9334352

<Icon

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 82 in 9334352

<Text

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 88 in 9334352

<Icon

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

To save the scroll position we can do the same way as we do in InitialSettingPage by using ScrollOffsetContext to store the scroll offset here, here and here. The route we pass to saveScrollOffset will be ROUTES.SEARCH_CENTRAL_PANE.route because we should use this for all search queries

const scrollViewRef = useRef<RNScrollView>(null);
const route = useRoute();
const {saveScrollOffset, getScrollOffset} = useContext(ScrollOffsetContext);

const onScroll = useCallback<NonNullable<ScrollViewProps['onScroll']>>(
    (e) => {
        // If the layout measurement is 0, it means the flashlist is not displayed but the onScroll may be triggered with offset value 0.
        // We should ignore this case.
        if (e.nativeEvent.layoutMeasurement.width === 0) {
            return;
        }
        saveScrollOffset(route, e.nativeEvent.contentOffset.x);
    },
    [saveScrollOffset, route],
);

useLayoutEffect(() => {
    const scrollOffset = getScrollOffset(route);
    if (!scrollOffset || !scrollViewRef.current) {
        return;
    }
    scrollViewRef.current.scrollTo({x: scrollOffset, animated: false});
}, [getScrollOffset, route]);

return (
    <ScrollView
        style={[styles.flexRow, styles.mb5, styles.overflowScroll, styles.flexGrow0]}
        horizontal
        onScroll={onScroll}
        ref={scrollViewRef}
        scrollEventThrottle={16}
        showsHorizontalScrollIndicator={false}
    >

App/src/components/Search/SearchStatusBar.tsx

Line 131 in d7c9087

<ScrollView

Update the font-size of the Expense button here by adding small prop for the icon and styles.fontSizeLabel for the title

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 78 in 9334352

<Icon

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 82 in 9334352

<Text

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Line 88 in 9334352

<Icon

What alternative solutions did you explore? (Optional)

NA

Result

Screen.Recording.2024-08-20.at.00.01.38.mov

nkdengineer · 2024-08-19T17:02:39Z

Updated proposal

Add detail to save scroll position and add result

melvin-bot · 2024-08-19T21:51:24Z

⚠️ Looks like this issue was linked to a Deploy Blocker here

If you are the assigned CME please investigate whether the linked PR caused a regression and leave a comment with the results.

If a regression has occurred and you are the assigned CM follow the instructions here.

If this regression could have been avoided please consider also proposing a recommendation to the PR checklist so that we can avoid it in the future.

bernhardoj · 2024-08-20T03:54:55Z

Edited by proposal-police: This proposal was edited at 2024-08-20 11:24:03 UTC.

Proposal

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

We want to fix 2 things.

Update the Expense label and icon size
Keep the scroll position for the search type when selecting the type

What is the root cause of that problem?

When we select the search type, for example Paid, the page loading which only renders the header and loading skeleton,

App/src/components/Search/index.tsx

Lines 195 to 206 in 6334d36

    
           if (shouldShowLoadingState) { 
        
               return ( 
        
                   <> 
        
                       <SearchPageHeader 
        
                           isCustomQuery={isCustomQuery} 
        
                           queryJSON={queryJSON} 
        
                           hash={hash} 
        
                       /> 
        
                       <SearchRowSkeleton shouldAnimate /> 
        
                   </> 
        
               ); 
        
           }

which means the SearchStatusBar is unmounted.

App/src/components/Search/index.tsx

Lines 314 to 329 in 6334d36

    
           return ( 
        
               <> 
        
                   <SearchPageHeader 
        
                       isCustomQuery={isCustomQuery} 
        
                       queryJSON={queryJSON} 
        
                       hash={hash} 
        
                       onSelectDeleteOption={handleOnSelectDeleteOption} 
        
                       data={data} 
        
                       setOfflineModalOpen={() => setOfflineModalVisible(true)} 
        
                       setDownloadErrorModalOpen={() => setDownloadErrorModalVisible(true)} 
        
                   /> 
        
                   <SearchStatusBar 
        
                       type={type} 
        
                       status={status} 
        
                   /> 
        
                   <SelectionListWithModal<ReportListItemType | TransactionListItemType>

and when the loading is completed, a new SearchStatusBar instance is mounted.

For the styling issue, it's just we don't apply the correct styling.

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

To keep the SearchStatusBar mounted even when loading, we need to add SearchStatusBar when loading too.

App/src/components/Search/index.tsx

Lines 195 to 203 in 6334d36

    
           if (shouldShowLoadingState) { 
        
               return ( 
        
                   <> 
        
                       <SearchPageHeader 
        
                           isCustomQuery={isCustomQuery} 
        
                           queryJSON={queryJSON} 
        
                           hash={hash} 
        
                       /> 
        
                       <SearchRowSkeleton shouldAnimate />

<SearchPageHeader />
<SearchStatusBar />
<SearchRowSkeleton />

Just like we did with an empty view too.

App/src/components/Search/index.tsx

Lines 220 to 233 in 6334d36

    
           if (shouldShowEmptyState) { 
        
               return ( 
        
                   <> 
        
                       <SearchPageHeader 
        
                           isCustomQuery={isCustomQuery} 
        
                           queryJSON={queryJSON} 
        
                           hash={hash} 
        
                       /> 
        
                       <SearchStatusBar 
        
                           type={type} 
        
                           status={status} 
        
                       /> 
        
                       <EmptySearchView type={type} /> 
        
                   </>

This way, the component tree for the header and status bar stays the same which prevents it from being unmounted.

|
|-Header
|-StatusBar
|-Skeleton/EmptyView/SelectionList

For the Expenses button style issue, the button height is actually 42 instead of 40. We can update the pressable and view height here to 40.

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Lines 67 to 76 in 6334d36

    
           <PressableWithFeedback 
        
               accessible 
        
               accessibilityLabel={popoverMenuItems[activeItemIndex]?.text ?? ''} 
        
               ref={buttonRef} 
        
               style={[styles.tabSelectorButton, styles.ph5]} 
        
               wrapperStyle={styles.flex1} 
        
               onPress={openMenu} 
        
           > 
        
               {({hovered}) => ( 
        
                   <Animated.View style={[styles.tabSelectorButton, styles.tabBackground(hovered, true, theme.border), styles.w100, styles.mh3]}>

And to make the icon 16px, we need to pass small or medium and hasText props to the Icon. And for the text, we need to pass styles.fontSizeLabel style so the font size is 13px.

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Lines 78 to 91 in 6334d36

    
           <Icon 
        
               src={menuIcon} 
        
               fill={theme.icon} 
        
           /> 
        
           <Text 
        
               numberOfLines={1} 
        
               style={[styles.textStrong, styles.flexShrink1]} 
        
           > 
        
               {menuTitle} 
        
           </Text> 
        
           <Icon 
        
               src={Expensicons.DownArrow} 
        
               fill={theme.icon} 
        
           />

Alternatively, we can replace the pressable with ButtonWithDropdownMenu. So, we can replace

App/src/pages/Search/SearchTypeMenuNarrow.tsx

Lines 67 to 103 in 6334d36

    
           <PressableWithFeedback 
        
               accessible 
        
               accessibilityLabel={popoverMenuItems[activeItemIndex]?.text ?? ''} 
        
               ref={buttonRef} 
        
               style={[styles.tabSelectorButton, styles.ph5]} 
        
               wrapperStyle={styles.flex1} 
        
               onPress={openMenu} 
        
           > 
        
               {({hovered}) => ( 
        
                   <Animated.View style={[styles.tabSelectorButton, styles.tabBackground(hovered, true, theme.border), styles.w100, styles.mh3]}> 
        
                       <View style={[styles.flexRow, styles.gap2, styles.alignItemsCenter, titleViewStyles]}> 
        
                           <Icon 
        
                               src={menuIcon} 
        
                               fill={theme.icon} 
        
                           /> 
        
                           <Text 
        
                               numberOfLines={1} 
        
                               style={[styles.textStrong, styles.flexShrink1]} 
        
                           > 
        
                               {menuTitle} 
        
                           </Text> 
        
                           <Icon 
        
                               src={Expensicons.DownArrow} 
        
                               fill={theme.icon} 
        
                           /> 
        
                       </View> 
        
                   </Animated.View> 
        
               )} 
        
           </PressableWithFeedback> 
        
           <PopoverMenu 
        
               menuItems={popoverMenuItems} 
        
               isVisible={isPopoverVisible} 
        
               anchorPosition={styles.createMenuPositionSidebar(windowHeight)} 
        
               onClose={closeMenu} 
        
               onItemSelected={closeMenu} 
        
               anchorRef={buttonRef} 
        
           />

with

<ButtonWithDropdownMenu
    onPress={() => null}
    shouldAlwaysShowDropdownMenu
    buttonSize={CONST.DROPDOWN_BUTTON_SIZE.MEDIUM}
    options={popoverMenuItems}
    isSplitButton={false}
    icon={menuIcon}
    style={[styles.ph5]}
    wrapperStyle={styles.flex1}
/>

To scroll into the selected status every time we refresh or deeplink to the page, we can scroll to the button position when the component is first mounted.

First, we can use onLayout, which we pass to the button. The Button needs to receive the onLayout props and pass it to the pressable.

onLayout={(e) => {
    if (!isActive || isScrolledRef.current) {
        return
    }
    isScrolledRef.current = true;
    ref.current?.scrollTo({x: e.nativeEvent.layout.left});
}}

If the item isn't selected (isActive is false), then we don't scroll to that item. Also, we only want this to happen once, so we use isScrolledRef to keep track of that.

rayane-d · 2024-08-20T10:56:42Z

Hi @nkdengineer and @bernhardoj,

In this PR, we will be adding a loading skeleton to the status bar. According to this discussion, we will keep the status bar mounted while changing the status. The status bar skeleton will only be shown while loading or changing the type (e.g., expense, invoice, trip).

Regarding the status scroll issue, We need to ensure that the selected status button (e.g., Paid status button) remains visible if we deeplink to a URL like https://dev.new.expensify.com:8082/search?q=type%3Aexpense%20status%3Apaid or when the page is refreshed.

Could you please update your proposals based on these details?

bernhardoj · 2024-08-20T11:24:59Z

Updated my proposal to scroll to the selected status bar.

rayane-d · 2024-08-21T00:35:59Z

We need to put this on hold for #47753 and #47515 as #47181 was reverted

rayane-d · 2024-08-23T19:43:03Z

Still on hold for #47753 and #47515

rayane-d · 2024-08-26T13:22:09Z

same ^

melvin-bot · 2024-09-09T19:59:20Z

Current assignee @rayane-djouah is eligible for the External assigner, not assigning anyone new.

bernhardoj · 2024-09-10T14:44:48Z

PR is ready

cc: @rayane-djouah

melvin-bot · 2024-09-20T18:05:21Z

@luacmartins, @mallenexpensify, @bernhardoj, @rayane-djouah Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

melvin-bot · 2024-09-21T18:00:03Z

Payment Summary

Upwork Job

Reviewer: @bernhardoj owed $250 via NewDot
ROLE: @rayane-djouah paid $(AMOUNT) via Upwork (LINK)

BugZero Checklist (@mallenexpensify)

I have verified the correct assignees and roles are listed above and updated the neccesary manual offers
I have verified that there are no duplicate or incorrect contracts on Upwork for this job (https://www.upwork.com/ab/applicants/1833233716363835239/hired)
I have paid out the Upwork contracts or cancelled the ones that are incorrect
I have verified the payment summary above is correct

luacmartins · 2024-09-23T00:33:26Z

Payment due

bernhardoj · 2024-09-23T04:26:29Z

Requested in ND.

JmillsExpensify · 2024-09-23T07:11:43Z

@mallenexpensify can I get a payment summary?

mallenexpensify · 2024-09-23T15:41:31Z

Contributor: @bernhardoj owed $250 via Upwork
Contributor+: @rayane-djouah paid $250 via NewDot

@bernhardoj can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021833233716363835239

@rayane-djouah plz complete the BZ checklist.

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

[@] The PR that introduced the bug has been identified. Link to the PR:
[@] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
[@] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
[@] Determine if we should create a regression test for this bug.
[@] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
[@bz] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

bernhardoj · 2024-09-23T15:51:35Z

@mallenexpensify I've requested the payment via ND.

JmillsExpensify · 2024-09-23T19:06:22Z

$250 approved for @bernhardoj

rayane-d · 2024-09-23T20:38:24Z

Checklist

The PR that introduced the bug has been identified. Link to the PR: This is a follow-up to this PR: Create status bar #47181 to address these comments: Create status bar #47181 (comment), Create status bar #47181 (comment)
The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment: The author and reviewers are aware already: Create status bar #47181 (comment)
A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion: N/A
Determine if we should create a regression test for this bug. I propose a regression test for the status bar auto-scrolling issue; the styling part will be handled during project wrap-up.
If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.

Regression Test Proposal

On mWeb Safari/Chrome:

Open the Search page.
Switch to "Paid" status on the status bar.
Refresh the page.
Verify that the status bar scrolls to the "Paid" button and the button is fully visible.

Do we agree 👍 or 👎

rayane-d · 2024-09-23T20:40:16Z

@mallenexpensify - Applied to the Upwork job

mallenexpensify · 2024-09-23T22:14:36Z

@rayane-djouah can you please accept the job and reply here once you have?
https://www.upwork.com/jobs/~021833233716363835239

Sorry, I thought I 'hired' you earlier, not just invited you.

Test case created

https://github.com/Expensify/Expensify/issues/430448

rayane-d · 2024-09-23T22:17:06Z

@mallenexpensify - Accepted, thank you!

mallenexpensify · 2024-09-23T22:20:29Z

Thanks @rayane-djouah , you're paid, payment comment updated above.

luacmartins added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 19, 2024

luacmartins self-assigned this Aug 19, 2024

luacmartins added this to [#whatsnext] #expense Aug 19, 2024

luacmartins added the External Added to denote the issue can be worked on by a contributor label Aug 19, 2024

melvin-bot bot assigned mallenexpensify Aug 19, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 19, 2024

melvin-bot bot assigned mananjadhav Aug 19, 2024

luacmartins mentioned this issue Aug 19, 2024

Create status bar #47181

Merged

60 tasks

luacmartins assigned rayane-d and unassigned mananjadhav Aug 19, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Aug 19, 2024

rayane-d mentioned this issue Aug 19, 2024

Search - Status bar scrolls back after selecting Paid status #47674

Closed

6 tasks

rayane-d mentioned this issue Aug 23, 2024

Create status bar v2 #47753

Merged

60 tasks

melvin-bot bot added the Overdue label Aug 23, 2024

melvin-bot bot added Overdue and removed Overdue labels Aug 23, 2024

melvin-bot bot removed the Overdue label Aug 26, 2024

bernhardoj mentioned this issue Sep 10, 2024

Scroll into view the search status button #48898

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 10, 2024

luacmartins changed the title ~~[$250] [Search v2.2] Keep status scroll position and update label sizes~~ [HOLD for payment 2024-09-21][$250] [Search v2.2] Keep status scroll position and update label sizes Sep 16, 2024

luacmartins added Awaiting Payment Auto-added when associated PR is deployed to production Daily KSv2 and removed Reviewing Has a PR in review Weekly KSv2 labels Sep 16, 2024

luacmartins moved this from Polish to Done in [#whatsnext] #expense Sep 16, 2024

melvin-bot bot added the Overdue label Sep 19, 2024

melvin-bot bot removed the Overdue label Sep 23, 2024

melvin-bot bot added Daily KSv2 and removed Daily KSv2 labels Sep 23, 2024

mallenexpensify closed this as completed Sep 23, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-09-21][$250] [Search v2.2] Keep status scroll position and update label sizes #47640

[HOLD for payment 2024-09-21][$250] [Search v2.2] Keep status scroll position and update label sizes #47640

luacmartins commented Aug 19, 2024 •

edited

Loading

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

nkdengineer commented Aug 19, 2024 •

edited

Loading

nkdengineer commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

bernhardoj commented Aug 20, 2024 •

edited by github-actions bot

Loading

rayane-d commented Aug 20, 2024

bernhardoj commented Aug 20, 2024

rayane-d commented Aug 21, 2024

rayane-d commented Aug 23, 2024

rayane-d commented Aug 26, 2024

melvin-bot bot commented Sep 9, 2024

bernhardoj commented Sep 10, 2024

melvin-bot bot commented Sep 20, 2024

melvin-bot bot commented Sep 21, 2024

luacmartins commented Sep 23, 2024

bernhardoj commented Sep 23, 2024

JmillsExpensify commented Sep 23, 2024

mallenexpensify commented Sep 23, 2024 •

edited

Loading

bernhardoj commented Sep 23, 2024

JmillsExpensify commented Sep 23, 2024

rayane-d commented Sep 23, 2024

rayane-d commented Sep 23, 2024

mallenexpensify commented Sep 23, 2024

rayane-d commented Sep 23, 2024

mallenexpensify commented Sep 23, 2024

[HOLD for payment 2024-09-21][$250] [Search v2.2] Keep status scroll position and update label sizes #47640

[HOLD for payment 2024-09-21][$250] [Search v2.2] Keep status scroll position and update label sizes #47640

Comments

luacmartins commented Aug 19, 2024 • edited Loading

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

melvin-bot bot commented Aug 19, 2024

nkdengineer commented Aug 19, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

Result

nkdengineer commented Aug 19, 2024

Updated proposal

melvin-bot bot commented Aug 19, 2024

bernhardoj commented Aug 20, 2024 • edited by github-actions bot Loading

Proposal

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

What is the root cause of that problem?

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

rayane-d commented Aug 20, 2024

bernhardoj commented Aug 20, 2024

rayane-d commented Aug 21, 2024

rayane-d commented Aug 23, 2024

rayane-d commented Aug 26, 2024

melvin-bot bot commented Sep 9, 2024

bernhardoj commented Sep 10, 2024

melvin-bot bot commented Sep 20, 2024

melvin-bot bot commented Sep 21, 2024

Payment Summary

BugZero Checklist (@mallenexpensify)

luacmartins commented Sep 23, 2024

bernhardoj commented Sep 23, 2024

JmillsExpensify commented Sep 23, 2024

mallenexpensify commented Sep 23, 2024 • edited Loading

bernhardoj commented Sep 23, 2024

JmillsExpensify commented Sep 23, 2024

rayane-d commented Sep 23, 2024

Checklist

rayane-d commented Sep 23, 2024

mallenexpensify commented Sep 23, 2024

rayane-d commented Sep 23, 2024

mallenexpensify commented Sep 23, 2024

luacmartins commented Aug 19, 2024 •

edited

Loading

nkdengineer commented Aug 19, 2024 •

edited

Loading

bernhardoj commented Aug 20, 2024 •

edited by github-actions bot

Loading

mallenexpensify commented Sep 23, 2024 •

edited

Loading