[$500] Background image crop after resize

@namhihi237

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.80-1
Reproducible in staging?: Yes
Reproducible in production?: Yes
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: @namhihi237
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1696951102341389

Action Performed:

Open a chat with any user
Click avatar to open RHN
Resize browser to smaller and back original

Expected Result:

The background image is not partially cropped

Actual Result:

The background image is partially cropped

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Screen.Recording.2023-10-10.at.22.19.47.1.mov

Background.mp4

MacOS: Desktop

Screen.Recording.2023-10-10.at.22.18.59.mov

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~015433ddd90da9816b
Upwork Job ID: 1711859135278239744
Last Price Increase: 2023-10-10

@trjExpensify

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

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

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

@fedirjh

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

@namhihi237

Proposal by @namhihi237

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

The background image is not partially cropped after resizing the screen

What is the root cause of that problem?

When opening RHN and resizing the screen background image, the part that RHN occupies will be lost here

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

we use a state variable forceRender to trigger a re-render of the component when the screen is focused.
When the screen is focused, we increment the forceRender variable, which causes the component to re-render with a different key.
This forces React to unmount and remount the component, effectively triggering a re-render of the AnimatedEmptyStateBackground component when the screen receives focus.
We update here

 const [forceRender, setForceRender] = useState(0);
 const isFocused = useIsFocused();
    useEffect(() => {
        if (!isFocused) {
            return;
        }
        setForceRender((prev) => prev + 1);
      }, [isFocused]);

We add key

        <Animated.Image
            key={forceRender}

What alternative solutions did you explore? (Optional)

I can repro this:

@namhihi237

@namhihi237 Can you elaborate more on the root cause?

Proposal

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

The background image is partially cropped after resizing the screen at the point when isSmallScreenWidth is true

What is the root cause of that problem?
useAnimatedStyle doesnt have a deps array. The isSmallScreenWidth state must be present in its deps array.

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

Lines 20 to 35 in 1998fa3

    
               const animatedStyles = useAnimatedStyle(() => { 
        
                   if (!isSmallScreenWidth) { 
        
                       return {}; 
        
                   } 
        
                   /* 
        
                    * We use x and y gyroscope velocity and add it to position offset to move background based on device movements. 
        
                    * Position the phone was in while entering the screen is the initial position for background image. 
        
                    */ 
        
                   const {x, y} = animatedSensor.sensor.value; 
        
                   // The x vs y here seems wrong but is the way to make it feel right to the user 
        
                   xOffset.value = NumberUtils.clampWorklet(xOffset.value + y, -IMAGE_OFFSET_X, IMAGE_OFFSET_X); 
        
                   yOffset.value = NumberUtils.clampWorklet(yOffset.value - x, -IMAGE_OFFSET_Y, IMAGE_OFFSET_Y); 
        
                   return { 
        
                       transform: [{translateX: withSpring(-IMAGE_OFFSET_X - xOffset.value)}, {translateY: withSpring(yOffset.value)}], 
        
                   }; 
        
               });

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

isSmallScreenWidth state must be added to the deps array`.
transition worked properly when provided with proper end transition styles

if (!isSmallScreenWidth) {
-       return {};
+      return {transform: [{translateX: withSpring(0)}, {translateY: withSpring(0)}]};
}

POC

I have fixed this issue. Please view the POC video

2023-10-11.06-58-35.mp4

Root cause demo using Devtools

Before applying the fix, the styles were not properly mutating/changing:

before.mp4

After applying the fix, the styles mutated/changed correctly:

after.mp4

Proposal

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

The background image is partially cropped when resizing.

What is the root cause of that problem?

We don't need the animated style for case isSmallScreenWidth=false (see here), but when resizing from isSmallScreenWidth=true to isSmallScreenWidth=false, the last transition state before the switch will still remain (as a behavior of Animated.Image), causing the issue.

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

We don't need animated style for isSmallScreenWidth=false as can be seen above, so we should use regular Image component in that case, also to reduce overhead since Animated.Image adds a lot of overhead for animation manipulation so we should only use it when absolutely necessary.

We can add here:

const ImageComponent = isSmallScreenWidth ? Animated.Image : Image;

And replace this with ImageComponent.

What alternative solutions did you explore? (Optional)

We can update this line to

return {transform: [{translateX: 0}, {translateY: 0}]};

or

return {transform: [{translateX: withSpring(0)}, {translateY: withSpring(0)}]};

to force it to transform to 0, but I prefer the main suggested approach since it also optimizes performance.

@trjExpensify

@trjExpensify This bug seems to be fixed by #26078 , not reproduced on staging.

@trjExpensify

@trjExpensify, @fedirjh Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

Ah yeah, same. I can't repro it anymore since that got deployed. We can close this issue out then! :)

Navigation Menu

[$500] Background image crop after resize #29237

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Bug0 Triage Checklist (Main S/O)

Proposal by @namhihi237

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)

Proposal

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

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

POC

Root cause demo using Devtools

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)

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	const animatedStyles = useAnimatedStyle(() => {
	if (!isSmallScreenWidth) {
	return {};
	}
	/*
	* We use x and y gyroscope velocity and add it to position offset to move background based on device movements.
	* Position the phone was in while entering the screen is the initial position for background image.
	*/
	const {x, y} = animatedSensor.sensor.value;
	// The x vs y here seems wrong but is the way to make it feel right to the user
	xOffset.value = NumberUtils.clampWorklet(xOffset.value + y, -IMAGE_OFFSET_X, IMAGE_OFFSET_X);
	yOffset.value = NumberUtils.clampWorklet(yOffset.value - x, -IMAGE_OFFSET_Y, IMAGE_OFFSET_Y);
	return {
	transform: [{translateX: withSpring(-IMAGE_OFFSET_X - xOffset.value)}, {translateY: withSpring(yOffset.value)}],
	};
	});

[$500] Background image crop after resize #29237

Description

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

Activity

melvin-bot commented on Oct 10, 2023

melvin-bot commented on Oct 10, 2023

melvin-bot commented on Oct 10, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot commented on Oct 10, 2023

m-natarajan commented on Oct 10, 2023

Proposal by @namhihi237

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)

trjExpensify commented on Oct 10, 2023

fedirjh commented on Oct 11, 2023

ashuvssut commented on Oct 11, 2023

Proposal

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

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

POC

Root cause demo using Devtools

dukenv0307 commented on Oct 11, 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)

fedirjh commented on Oct 13, 2023

melvin-bot commented on Oct 16, 2023

trjExpensify commented on Oct 16, 2023

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions