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

[Sharing] Fix Tumblr form being pushed down #18289

Merged
merged 1 commit into from
Apr 20, 2023

Conversation

thomashorta
Copy link
Contributor

@thomashorta thomashorta commented Apr 19, 2023

Fixes #17401

This fixes the bug reported in this comment in the original issue.

The NestedScrollView behavior paired with the WebView was causing the WebView height to increase indefinitely while the Tumblr login was trying to center its content vertically, causing the login form to be pushed down.

This likely didn't happen with other services because they do not center their content vertically or at least don't do it the same way Tumblr was doing.

The correct solution though was to use the NestedWebView, which was created to avoid this kind of scroll issue that can affect WebViews inside scrolling layouts.

fix_runaway_tumblr_login.mp4

To test:

  1. Open Jetpack
  2. Login with a WP.com account
  3. Go to My Site > Menu > Sharing
  4. Select Tumblr
  5. Tap Connect
  6. 🔎 Verify the page loads correctly and scrolls normally, if needed

Extra verification: check that the login page for the other services works as expected as well.

Regression Notes

  1. Potential unintended areas of impact
    Connection pages for other services are affected.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    Manual testing.

  3. What automated tests I added (or what prevented me from doing so)
    N/A, UI only fix.

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

  • Portrait and landscape orientations.
  • Light and dark modes.
  • Fonts: Larger, smaller and bold text.
  • High contrast.
  • Talkback.
  • Languages with large words or with letters/accents not frequently used in English.
  • Right-to-left languages. (Even if translation isn’t complete, formatting should still respect the right-to-left layout)
  • Large and small screen sizes. (Tablet and smaller phones)
  • Multi-tasking: Split screen and Pop-up view. (Android 10 or higher)

Note: tested with RTL and Talkback, but since this issue refers to a WebView, I am not assessing the content behavior for this.

The NestedScrollView behavior paired with the WebView was causing the webview
height to increase indefinitely while the tumblr login was trying to center its
content vertically, causing the login form to be pushed down.

This likely didn't happen with other services because they do not center their
content vertically or at least don't do it the same way tumblr was doing.

The correct solution though was to use the NestedWebView, which was created to
avoid these kind of scroll issues that can affect WebViews inside scrolling
layouts.
@thomashorta thomashorta added this to the 22.3 milestone Apr 19, 2023
@thomashorta thomashorta requested a review from RenanLukas April 19, 2023 20:47
@thomashorta thomashorta self-assigned this Apr 19, 2023
@thomashorta thomashorta requested a review from hafizrahman April 19, 2023 20:48
@wpmobilebot
Copy link
Contributor

Jetpack📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.
App NameJetpack Jetpack
FlavorJalapeno
Build TypeDebug
Versionpr18289-1dd9d6c
Commit1dd9d6c
Direct Downloadjetpack-prototype-build-pr18289-1dd9d6c.apk
Note: Google Login is not supported on these builds.

@wpmobilebot
Copy link
Contributor

WordPress📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.
App NameWordPress WordPress
FlavorJalapeno
Build TypeDebug
Versionpr18289-1dd9d6c
Commit1dd9d6c
Direct Downloadwordpress-prototype-build-pr18289-1dd9d6c.apk
Note: Google Login is not supported on these builds.

Copy link
Contributor

@RenanLukas RenanLukas left a comment

Choose a reason for hiding this comment

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

Thanks for fixing it, @thomashorta. It's working as expected.
LGTM :shipit:

@thomashorta thomashorta merged commit 2ed867f into trunk Apr 20, 2023
@thomashorta thomashorta deleted the issue/17401-sharing-connection-tumblr branch April 20, 2023 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sharing: Tumblr connection process doesn't load
3 participants