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

Fix visible scroll bars on web and non scrollable content on mWeb #2148

Merged
merged 3 commits into from
Mar 30, 2021

Conversation

marcaaron
Copy link
Contributor

@marcaaron marcaaron commented Mar 30, 2021

Details

Overflow scroll creates weird ugly scrollbars on web and modals are not scrolling properly cc @NikkiWines

Also, undoes the height: 0 trick we used here #2101 since it's no longer needed after the other changes.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/158769
Fixes #2147

Tests

  1. Navigate to /settings/profile & /settings/password
  2. Verify there are no visible scrollbars on web views with content that does not overflow
  3. Verify the screen can be scroll on smaller screen mobile views if the content overflows

Test New Group Button appears

  1. Navigate to New Group screen
  2. Verify options can be scrolled
  3. Select 4 options
  4. Verify the Create Group button appears

Screen Shot 2021-03-30 at 8 48 04 AM

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

Web

2021-03-29_14-39-33
2021-03-29_14-39-27

Mobile Web

2021-03-29_16-12-37.mp4

Desktop

2021-03-29_16-57-35.mp4

iOS

2021-03-29_16-44-11

Android

2021-03-29_16-56-14

@marcaaron marcaaron self-assigned this Mar 30, 2021
@marcaaron marcaaron requested a review from a team as a code owner March 30, 2021 00:38
@MelvinBot MelvinBot requested review from bondydaa and removed request for a team March 30, 2021 00:39
@marcaaron marcaaron marked this pull request as draft March 30, 2021 00:43
@marcaaron
Copy link
Contributor Author

Tossing this back into draft for a second mWeb seems kinda broken in simulator

@marcaaron marcaaron requested a review from jasperhuangg March 30, 2021 02:53
@marcaaron marcaaron marked this pull request as ready for review March 30, 2021 02:58
@marcaaron
Copy link
Contributor Author

@jasperhuangg adding you as a reviewer here since I think this is the proper fix for the issues fixed by #2101, but lmk if I'm missing anything.

@marcaaron marcaaron changed the title Fix overflow scroll issue Fix visible scroll bars on web and non scrollable content on mWeb Mar 30, 2021
@jasperhuangg
Copy link
Contributor

@marcaaron Looks good! I tested it out on my end and verified everything works great, including the inability to scroll that was addressed in the past issue.

@shawnborton
Copy link
Contributor

Another place to check for testing - does this work in the "New Group" flow where after selecting a person, the "Create Group" button is added to the bottom of the screen?

@bondydaa
Copy link
Contributor

I'm confused... in your videos I do see scroll bars

image

Copy link
Contributor

@bondydaa bondydaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code changes seem bueno 👍

@marcaaron
Copy link
Contributor Author

@shawnborton yes it works, but I'll add it as a testing step for good measure

@marcaaron
Copy link
Contributor Author

I'm confused... in your videos I do see scroll bars

@bondydaa Updated the tests steps but see the original issue, it's confusing as there are two issues

  1. Screens that should scroll are not able to scroll at all
  2. On web, screens that should not scroll (no content overflow) non functional scrollbars are shown

@bondydaa bondydaa merged commit 6dc9c81 into master Mar 30, 2021
@bondydaa bondydaa deleted the marcaaron-fixOverflowIssue branch March 30, 2021 19:04
@OSBotify
Copy link
Contributor

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

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.

iOS/Android - Profile page does not scroll
5 participants