Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[$500] LHN - White line between LHN and chat view when the webpage zoom level is 75% #32745

Closed lanitochka17 closed 10 months ago

lanitochka17 commented 11 months ago

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.4.10-0 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: Applause - Internal Team Slack conversation:

Action Performed:

  1. Go to staging.new.expensify.com.
  2. Decrease webpage zoom level to 75%.

Expected Result:

There is no visual issue involving white element

Actual Result:

The line between LHN and chat view becomes white when the webpage zoom level is 75%

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/6cf75338-72de-4f91-b851-47f3cce5490c

Bug6306186_1702063920544!bandicam_2023-12-09_03-29-06-183

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~012343f6a46c0a8e3f
  • Upwork Job ID: 1733215408636047360
  • Last Price Increase: 2023-12-22
melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

Bug0 Triage Checklist (Main S/O)

melvin-bot[bot] commented 11 months ago

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

happy-devs commented 11 months ago

This also happens on 110% / 50% zoom

happy-devs commented 11 months ago

Proposal

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

The line between LHN and chat view becomes white when the webpage zoom level is 75%

What is the root cause of that problem?

The css class box-sizing: border-box inside the view class is causing the issue. once removed it's no longer there

https://github.com/Expensify/App/assets/78056086/257835b0-780e-4c08-9fee-772123c7868c

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

EDIT: after some research I found this is a feature of React Native itself - the View component has box-sizing packed into it and people are finding all kinds of solution around this fix. Not sure how the internal team wants to tackle this though

https://stackoverflow.com/questions/38503451/does-it-exist-an-equivalent-of-box-sizing-border-box-in-flexbox-for-react-nativ

ZhenjaHorbach commented 11 months ago

Proposal

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

White line between LHN and chat view when the webpage zoom leve

What is the root cause of that problem?

in fact, this is a problem related to sideBarWidth

https://github.com/Expensify/App/blob/adbeba13ef409abb11024a2bf602a6625d3249fb/src/styles/variables.ts#L87

Since our value is too specific It is very difficult to scale As a result, we have non-integer values for styles What causes white streaks to appear between components

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

The easiest way to fix this This is to change marginLeft for container style from an odd number to an even number Which won't affect anything visually But the bug will disappear

rootNavigatorContainerStyles: (isSmallScreenWidth: boolean) => ({marginLeft: isSmallScreenWidth ? 0 : variables.sideBarWidth - 5, flex: 1} satisfies ViewStyle),

https://github.com/Expensify/App/blob/271b0a5f26555920dcf518e5d12cc31608e3a0d1/src/styles/styles.ts#L2406

What alternative solutions did you explore? (Optional)

NA

https://github.com/Expensify/App/assets/68128028/3d186e1d-627c-4077-8243-0903b4f07df8

dukenv0307 commented 11 months ago

Proposal

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

The line between LHN and chat view becomes white when the webpage zoom level is 75%

What is the root cause of that problem?

There're inconsistency between how we're styling the sidebar screen and the root navigator. The sidebar is using the translateX to transform itself to the correct position here, while the root navigator use marginLeft here with the same variables.sideBarWidth value. When the zoom level of the browser is changed, the transformX and marginLeft are calculated proportionally by the browser and rounded differently, leading to different transformX and marginLeft final values, causing a white gap in between.

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

We simply need to make the styling consistent between the sidebar screen and the root navigator. We can use transformX for the root navigator as well. We can update this to

rootNavigatorContainerStyles: (isSmallScreenWidth: boolean) => ({
            transform: [{translateX: isSmallScreenWidth ? 0 : variables.sideBarWidth}],
            flex: 1
        } satisfies ViewStyle),

(Just changing marginLeft to the equivalent translateX to be consistent)

What alternative solutions did you explore? (Optional)

We should double check if there other inconsistencies relating to the sideBarWidth, if there are, can fix similarly.

shahinyan11 commented 11 months ago

https://github.com/Expensify/App/issues/31573

I think this two issues need to be considered together . They can interfere with each other

bernhardoj commented 11 months ago

I don't know if I see the same line or not, but that's the border of the sidebar https://github.com/Expensify/App/blob/632c5c6fb416e330cc90878b6258a9b5e23347cb/src/libs/Navigation/AppNavigator/getRootNavigatorScreenOptions.ts#L47

Ollyws commented 11 months ago

Seems like this will be fixed by @bernhardoj's PR: https://github.com/Expensify/App/pull/32772

Ollyws commented 11 months ago

Issue is no longer reproducible for me since the above PR was merged.

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 11 months ago

@Ollyws, @peterdbarkerUK Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 11 months ago

@Ollyws, @peterdbarkerUK Huh... This is 4 days overdue. Who can take care of this?

melvin-bot[bot] commented 11 months ago

@Ollyws, @peterdbarkerUK Still overdue 6 days?! Let's take care of this!

mvtglobally commented 11 months ago

Issue not reproducible during KI retests. (First week)

melvin-bot[bot] commented 11 months ago

@Ollyws @peterdbarkerUK this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

melvin-bot[bot] commented 11 months ago

@Ollyws, @peterdbarkerUK 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

melvin-bot[bot] commented 11 months ago

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

melvin-bot[bot] commented 10 months ago

@Ollyws, @peterdbarkerUK 12 days overdue now... This issue's end is nigh!

peterdbarkerUK commented 10 months ago

Not reproducible, closing