Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove scroll bar from home screen #8547

Merged
merged 1 commit into from
May 7, 2020
Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented May 7, 2020

A change made in #8284 had the unintended side-effect of making this scrollbar appear on the home screen. Previously it was scrollable without any scroll bar being visible.

A change made in #8284 had the unintended side-effect of making this
scrollbar appear on the home screen. Previously it was scrollable
without any scroll bar being visible.
@Gudahtt Gudahtt requested a review from a team as a code owner May 7, 2020 13:27
@Gudahtt
Copy link
Member Author

Gudahtt commented May 7, 2020

Screenshots: With scrollbar (before):

with-scrollbar

Without scrollbar (after):

without-scrollbar

@metamaskbot
Copy link
Collaborator

Builds ready [563309a]
Page Load Metrics (654 ± 35 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint309941178
domContentLoaded3747726537335
load3757736547335
domInteractive3737716527335

@brad-decker
Copy link
Contributor

brad-decker commented May 7, 2020

@Gudahtt is this issue caused by an overlap of the mentioned change and my updates in #8433 ?

@Gudahtt
Copy link
Member Author

Gudahtt commented May 7, 2020

@brad-decker I thought so at first too, but apparently not. I compared before and after #8433, and this particular scrollbar remained unchanged. I did a git bisect, and found that it was introduced in #8284.

@brad-decker
Copy link
Contributor

Sweet. How did you use git bisect with a visual regression? E2E test to check if the scrollbar was visible?

@Gudahtt
Copy link
Member Author

Gudahtt commented May 7, 2020

I just manually ran yarn then yarn start for each commit, and looked at the extension in the browser. Probably not the most efficient process 😅 But it works.

@brad-decker
Copy link
Contributor

Bet you spent less time doing that than it would have taken to write a test to look for a scrollbar, lol.

@Gudahtt Gudahtt merged commit 7a6b259 into develop May 7, 2020
@Gudahtt Gudahtt deleted the remove-home-screen-scrollbar branch May 7, 2020 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants