[C+ Checklist Needs Completion] [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly #30218

m-natarajan · 2023-10-23T22:21:17Z

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: 1.3.89-6
Reproducible in staging?: y
Reproducible in production?: y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1698091949515899

Action Performed:

Open a chat
Go to the request money section (Pay or delete request money if you've requested before)
Enter a specific amount and send the request money
Click on the requested money
Hover over the requested money and click "Mark as unread"
Observe the background image scrolling up and down

Expected Result:

Clicking on "Mark as unread" should not cause the background image to scroll up and down

Actual Result:

When clicking on "Mark as unread," the background image scrolls up and down unexpectedly

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Android: Native

20231024011931.mp4

Android: mWeb Chrome

20231024012134.mp4

iOS: Native

Screen.Recording.2023-10-24.at.1.17.53.AM.mov

iOS: mWeb Safari

Screen.Recording.2023-10-24.at.1.18.36.AM.mov

MacOS: Chrome / Safari

Screen.Recording.2023-10-24.at.1.13.42.AM.mov

Mark.as.unread.the.background.image.scrolls.up.and.down.unexpectedly.mp4

MacOS: Desktop

2023-10-24.01-05-39.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~011368eedf256e8dda
Upwork Job ID: 1716580589659369472
Last Price Increase: 2023-10-23
Automatic offers:

dukenv0307 | Contributor | 27499352

melvin-bot · 2023-10-23T22:21:22Z

Triggered auto assignment to @greg-schroeder (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot · 2023-10-23T22:21:23Z

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

melvin-bot · 2023-10-23T22:21:24Z

Bug0 Triage Checklist (Main S/O)

This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
This bug is not a duplicate report (check E/App issues and #expensify-bugs)
- If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
This bug is reproducible using the reproduction steps in the OP. S/O
- If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
- If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
This issue is filled out as thoroughly and clearly as possible
- Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

melvin-bot · 2023-10-23T22:21:29Z

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

akamefi202 · 2023-10-24T01:12:51Z

Proposal

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

When clicking on "Mark as unread" the background image scrolls up and down unexpectedly.

What is the root cause of that problem?

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 287 to 290 in 112061b

    
           <SpacerView 
        
               shouldShow={shouldShowHorizontalRule} 
        
               style={[shouldShowHorizontalRule ? styles.reportHorizontalRule : {}]} 
        
           />

SpacerView component represents the horizontal line(not unread marker) below money request view.
And it is shown or hidden by shouldShowHorizontalRule variable.

App/src/components/SpacerView.js

Line 36 in 112061b

    
           marginVertical: shouldShow ? CONST.HORIZONTAL_SPACER.DEFAULT_MARGIN_VERTICAL : CONST.HORIZONTAL_SPACER.HIDDEN_MARGIN_VERTICAL,

SpacerView component has 8px margin for both top & bottom(16px in total) and the gap is removed if the component is hidden.
And this gap makes scroll when the component is shown/hidden in animated way using react-native-reanimated library.

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

To remove scroll, we need to keep gap although SpacerView is hidden.

marginVertical: CONST.HORIZONTAL_SPACER.DEFAULT_MARGIN_VERTICAL,

App/src/pages/home/report/ReportActionItem.js

Line 610 in 112061b

    
           return <View style={[props.shouldHideThreadDividerLine && !isNormalCreatedAction && styles.mb2]}>{content}</View>;

We should remove margin style in above code, which add bottom margin if spacer is hidden.
If we keep the gap in spacer, we don't need to add margin here.

It's related to this PR and this PR might be a regression in this case.
#27766

I'm still not sure if we should prevent scroll at all. Because I think react-native-reanimated library is used for smooth scroll.
But at least, we need to fix one thing although we leave the scroll issue.

App/src/pages/home/report/ReportActionItem.js

Line 610 in 112061b

    
           return <View style={[props.shouldHideThreadDividerLine && !isNormalCreatedAction && styles.mb2]}>{content}</View>;

The spacer has 16px margin if it's shown, but the above code only adds 8px margin if the spacer is hidden.
We should use styles.mb4 instead.

What alternative solutions did you explore? (Optional)

N/A

dukenv0307 · 2023-10-24T03:46:02Z

Proposal

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

When clicking on "Mark as unread" the background image scrolls up and down unexpectedly

What is the root cause of that problem?

When we mark as unread the first visible action, the thread divider line is hidden, and marginVertical is changed here. So the created view scrolls up by Animated.View of react-native-reanimated lib

App/src/components/SpacerView.js

Lines 34 to 37 in d282473

    
           const duration = CONST.ANIMATED_TRANSITION; 
        
           const values = { 
        
               marginVertical: shouldShow ? CONST.HORIZONTAL_SPACER.DEFAULT_MARGIN_VERTICAL : CONST.HORIZONTAL_SPACER.HIDDEN_MARGIN_VERTICAL, 
        
               borderBottomWidth: shouldShow ? CONST.HORIZONTAL_SPACER.DEFAULT_BORDER_BOTTOM_WIDTH : CONST.HORIZONTAL_SPACER.HIDDEN_BORDER_BOTTOM_WIDTH,

And then because when the thread divider line is hidden we have a marginBottom here, the created view scrolls down.

App/src/pages/home/report/ReportActionItem.js

Line 610 in d282473

    
           return <View style={[props.shouldHideThreadDividerLine && !isNormalCreatedAction && styles.mb2]}>{content}</View>;

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

This margin-bottom adds a space between the created view and the unread marker. To avoid the scrolls up and down, we can
remove this margin-bottom and change HIDDEN_MARGIN_VERTICAL from 0 to 4 to keep the space between the created view and the unread marker

App/src/pages/home/report/ReportActionItem.js

Line 610 in d282473

    
           return <View style={[props.shouldHideThreadDividerLine && !isNormalCreatedAction && styles.mb2]}>{content}</View>;

App/src/CONST.ts

Line 2785 in d282473

HIDDEN_MARGIN_VERTICAL: 0,

What alternative solutions did you explore? (Optional)

We can change duration to 0 if shouldShow is false

const duration = shouldShow ? CONST.ANIMATED_TRANSITION : 0;

Or we can remove the use of animated view in SpacerView

melvin-bot · 2023-10-27T12:54:57Z

@greg-schroeder, @allroundexperts Whoops! This issue is 2 days overdue. Let's get this updated quick!

greg-schroeder · 2023-10-27T16:51:57Z

Awaiting proposal review

allroundexperts · 2023-10-30T03:03:15Z

@akamefi202's proposal looks good to me.

@dukenv0307 I think your proposal is almost the same as what @akamefi202 proposed earlier. Please let me know if I missed anything.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-10-30T03:03:21Z

Triggered auto assignment to @youssef-lr, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

dukenv0307 · 2023-10-30T03:55:58Z

@allroundexperts If the ideal is to keep the animated view, my solution is different from the selected solution since I suggested changing HIDDEN_MARGIN_VERTICAL from 0 to 4 to make the space between the created view and the divider and the space between the created view and the unread marker are the same and it's 8px, while the selected solution make the space between the unread marker and the created view is 16px that isn't consistent with the space between the divider and the create view.

akamefi202 · 2023-10-30T06:16:06Z

@dukenv0307 It's consistent because the proposal suggests to remove margin in ReportActionItem class.

App/src/pages/home/report/ReportActionItem.js

Line 610 in 112061b

    
           return <View style={[props.shouldHideThreadDividerLine && !isNormalCreatedAction && styles.mb2]}>{content}</View>;

dukenv0307 · 2023-10-30T06:26:35Z

@akamefi202 it's not consistent. Let's see the space between the last menu item and the divider it's 8px.

akamefi202 · 2023-10-30T06:44:09Z

@dukenv0307 8px margin is added here. #27766
I think it should have been 16px instead because SpacerView component has 8px margin for both top & bottom. So the gap is 16px in total.
And we should keep same top & bottom margin inside SpacerView component to prevent scrolling up & down.

dukenv0307 · 2023-10-30T06:54:04Z

I think it should be 16px instead because SpacerView component has 8px margin for both top & bottom. So the gap is 16px in total.

If we add a space of 16px, it's the space between the created view and the unread marker. It's wrong because it should be 8px.

And we should keep same top & bottom margin inside SpacerView component to prevent scrolling up & down.

We only need to prevent SpacerView from scrolling up and down at the same time because the unread marker has no space with the action, it's expected that the divider line will be scrolled down when the unread marker appears. That is the reason animated view is added here #27307 (comment)

cc @allroundexperts to share your thoughts.

akamefi202 · 2023-10-31T19:02:14Z

@greg-schroeder @allroundexperts Could you plz let me start working on this issue if there isn't other problem?

youssef-lr · 2023-11-01T23:16:06Z

@allroundexperts what's the final decision here?

allroundexperts · 2023-11-02T17:24:58Z

Going through the discussion again!

dukenv0307 · 2023-11-03T06:33:28Z

@allroundexperts The PR is ready for review.

erquhart · 2023-11-09T12:12:07Z

This is a regression from #27766

greg-schroeder · 2023-11-14T16:48:11Z

Reviews continue on linked PR

greg-schroeder · 2023-11-26T20:11:08Z

Bump @youssef-lr - please take a look at the linked PR/questions for you from C! Thanks!

#30815

melvin-bot · 2023-12-08T01:03:52Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2023-12-08T01:03:58Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.9-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

Fix created view is flickered when marking as unread #30815

If no regressions arise, payment will be issued on 2023-12-15. 🎊

After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.

External issue reporter
Contributor that fixed the issue
Contributor+ that helped on the issue and/or PR

For reference, here are some details about the assignees on this issue:

@allroundexperts requires payment through NewDot Manual Requests
@dukenv0307 requires payment automatic offer (Contributor)
@ayazhussain79 requires payment (Needs manual offer from BZ)

melvin-bot · 2023-12-08T01:03:59Z

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:

[@allroundexperts] The PR that introduced the bug has been identified. Link to the PR:
[@allroundexperts] 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:
[@allroundexperts] 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:
[@allroundexperts] Determine if we should create a regression test for this bug.
[@allroundexperts] 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.
[@greg-schroeder] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

greg-schroeder · 2023-12-15T22:00:58Z

Reporter: $50
Contributor: $500
C+: $500

The job post was closed, so I created a new payment issue and sent out new offers. Sibtain you can file a manual request.

Next up is the checklist!

greg-schroeder · 2023-12-18T16:59:10Z

@dukenv0307 offer still pending if you're able to accept!

greg-schroeder · 2023-12-19T02:34:24Z

Gentle bump on checklist as well @allroundexperts :)

dukenv0307 · 2023-12-19T05:14:27Z

@dukenv0307 offer still pending if you're able to accept!

@greg-schroeder I've accepted, thank you!

allroundexperts · 2023-12-19T12:37:18Z

Checklist

I was not able to pinpoint any particular PR which caused this issue. I think several PRs down the road might have caused this.
N/A
N/A
A regression test would be good here.

Regression test

Open a chat and request money.
Click on the requested money
Hover over the requested money and click "Mark as unread"

Verify that clicking on "Mark as unread" does not cause the background image to scroll up and down.

Do we 👍 or 👎 ?

greg-schroeder · 2023-12-21T19:09:58Z

LGTM. Adding and closing.

m-natarajan added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 23, 2023

melvin-bot bot assigned greg-schroeder Oct 23, 2023

melvin-bot bot changed the title ~~When clicking on "Mark as unread" the background image scrolls up and down unexpectedly~~ [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly Oct 23, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 23, 2023

melvin-bot bot assigned allroundexperts Oct 23, 2023

melvin-bot bot added the Overdue label Oct 26, 2023

melvin-bot bot removed the Overdue label Oct 27, 2023

melvin-bot bot assigned youssef-lr Oct 30, 2023

melvin-bot bot added the Overdue label Nov 1, 2023

melvin-bot bot removed the Overdue label Nov 1, 2023

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 3, 2023

erquhart mentioned this issue Nov 8, 2023

[$500] IOU - When we change category of manual request, a small shake can be seen #29877

Closed

6 tasks

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Dec 8, 2023

melvin-bot bot changed the title ~~[$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly~~ [HOLD for payment 2023-12-15] [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly Dec 8, 2023

melvin-bot bot removed the Reviewing Has a PR in review label Dec 8, 2023

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Dec 14, 2023

melvin-bot bot added the Overdue label Dec 18, 2023

melvin-bot bot removed the Overdue label Dec 18, 2023

melvin-bot bot added the Overdue label Dec 21, 2023

melvin-bot bot removed the Overdue label Dec 21, 2023

greg-schroeder closed this as completed Dec 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[C+ Checklist Needs Completion] [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly #30218

[C+ Checklist Needs Completion] [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly #30218

m-natarajan commented Oct 23, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 23, 2023

melvin-bot bot commented Oct 23, 2023

melvin-bot bot commented Oct 23, 2023

melvin-bot bot commented Oct 23, 2023

akamefi202 commented Oct 24, 2023 •

edited

Loading

dukenv0307 commented Oct 24, 2023

melvin-bot bot commented Oct 27, 2023

greg-schroeder commented Oct 27, 2023

allroundexperts commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

dukenv0307 commented Oct 30, 2023 •

edited

Loading

akamefi202 commented Oct 30, 2023

dukenv0307 commented Oct 30, 2023

akamefi202 commented Oct 30, 2023 •

edited

Loading

dukenv0307 commented Oct 30, 2023 •

edited

Loading

akamefi202 commented Oct 31, 2023

youssef-lr commented Nov 1, 2023

allroundexperts commented Nov 2, 2023

dukenv0307 commented Nov 3, 2023

erquhart commented Nov 9, 2023

greg-schroeder commented Nov 14, 2023

greg-schroeder commented Nov 26, 2023

melvin-bot bot commented Dec 8, 2023

melvin-bot bot commented Dec 8, 2023

melvin-bot bot commented Dec 8, 2023

greg-schroeder commented Dec 15, 2023

greg-schroeder commented Dec 18, 2023

greg-schroeder commented Dec 19, 2023

dukenv0307 commented Dec 19, 2023

allroundexperts commented Dec 19, 2023

greg-schroeder commented Dec 21, 2023

[C+ Checklist Needs Completion] [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly #30218

[C+ Checklist Needs Completion] [$500] When clicking on "Mark as unread" the background image scrolls up and down unexpectedly #30218

Comments

m-natarajan commented Oct 23, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 23, 2023

melvin-bot bot commented Oct 23, 2023

melvin-bot bot commented Oct 23, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Oct 23, 2023

akamefi202 commented Oct 24, 2023 • 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)

dukenv0307 commented Oct 24, 2023

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)

melvin-bot bot commented Oct 27, 2023

greg-schroeder commented Oct 27, 2023

allroundexperts commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

dukenv0307 commented Oct 30, 2023 • edited Loading

akamefi202 commented Oct 30, 2023

dukenv0307 commented Oct 30, 2023

akamefi202 commented Oct 30, 2023 • edited Loading

dukenv0307 commented Oct 30, 2023 • edited Loading

akamefi202 commented Oct 31, 2023

youssef-lr commented Nov 1, 2023

allroundexperts commented Nov 2, 2023

dukenv0307 commented Nov 3, 2023

erquhart commented Nov 9, 2023

greg-schroeder commented Nov 14, 2023

greg-schroeder commented Nov 26, 2023

melvin-bot bot commented Dec 8, 2023

melvin-bot bot commented Dec 8, 2023

melvin-bot bot commented Dec 8, 2023

greg-schroeder commented Dec 15, 2023

greg-schroeder commented Dec 18, 2023

greg-schroeder commented Dec 19, 2023

dukenv0307 commented Dec 19, 2023

allroundexperts commented Dec 19, 2023

Checklist

Regression test

greg-schroeder commented Dec 21, 2023

m-natarajan commented Oct 23, 2023 •

edited by melvin-bot bot

Loading

akamefi202 commented Oct 24, 2023 •

edited

Loading

dukenv0307 commented Oct 30, 2023 •

edited

Loading

akamefi202 commented Oct 30, 2023 •

edited

Loading

dukenv0307 commented Oct 30, 2023 •

edited

Loading