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

Webview fixes #73

Closed

Conversation

spinosa
Copy link

@spinosa spinosa commented Apr 8, 2022

  • Fix Swift compiler warning about PathConfigurationDelegate being a class by making it AnyObject
  • When activating the webview, insert at index 0 so the containing UINavigationController locates its scrollview and correctly handles scrolling related behavior (ex: navigation bar translucency)
  • Install UIRefreshControl on the .refreshControl property of the web view's scroll view
  • Add documentation about viewport-fit=cover affecting the position of the UIRefreshControl

UINavigationController does not correctly locate the webview's ScrollView
if the WebView is not the ViewController's lowest view
If a view `allowsPullToRefresh` in a turbo-ios native app with `viewport-fit=cover`,
the UIRefreshControl will not be positioned as expected.

You can fix this by conditionally removing `viewport-fit=cover` when loading in a native app.
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have a screenshot for this issue?

We set the viewport to width=device-width,initial-scale=1,maximum-scale=1,viewport-fit=cover and haven't noticed this issue.

Copy link
Author

Choose a reason for hiding this comment

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

There could be confounders at play. Would need to repro on a minimal app. But here are screenshots of our app (deployment target 14.0, running on 15.4.1) where the only difference is the inclusion of viewport-fit=cover:

Without viewport-fit=cover:
IMG_4551

With viewport-fit=cover:
IMG_4550

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahh you're right ours does that too, now I look at it. I was always like 🤷 that's high up but whatever.

@tabuna
Copy link

tabuna commented Sep 6, 2022

I faced the same issue. On a project without specifying cover:

image

But with indication, it looks like this (the slider is almost invisible due to the white bar):

image

Is there any way to get the correct display from the viewport cover?

@connorbey13
Copy link

This fixed my issue with content showing in the nav bar and status bar. 👍

@JoshAntBrown
Copy link

This fixed the same translucency issue around nav, status and tab bars but broke the pull down to dismiss modal interaction because the refresh control took over. Ended up creating a fork and creating a branch with just the change to insert the webview fixing the translucency issue.

@joemasilotti
Copy link
Member

Fix Swift compiler warning about PathConfigurationDelegate being a class by making it AnyObject

Nice, thanks for that!

When activating the webview, insert at index 0 so the containing UINavigationController locates its scrollview and correctly handles scrolling related behavior (ex: navigation bar translucency)

Install UIRefreshControl on the .refreshControl property of the web view's scroll view

Good call. @jayohms, is there any history as to why this wasn't done to begin with? We aren't introducing any weird edge cases?

Add documentation about viewport-fit=cover affecting the position of the UIRefreshControl

Confirmed that this fixes a lot of issues I've seen folks mention. In the second screenshot I'm pulling down to refresh and the spinner is in the correct position.

viewport-fit=cover (removed)
image image

@Bramjetten
Copy link
Contributor

How are you people handling the viewport-fit=cover bug? Without viewport-fit=cover, all of my pages are longer than the screen resulting in an unnecessary scroll area. Adding viewport-fit=cover fixes that, but it introduces this bug with the refreshControl.

@svara
Copy link
Contributor

svara commented Feb 26, 2024

This has been fixed in #123.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants