Reimplement Safari view

hbang / NewTerm

NewTerm is a terminal emulator app with first-class iPhone, iPad, and Mac support.

https://newterm.app

Other

463 stars 71 forks source link

Reimplement Safari view #62

Closed chrisharper22 closed 3 years ago

chrisharper22 commented 3 years ago

While this fix does serve its purpose for fixing the bottom edge insets, it does seem beneficial for the view's implementation itself to be redone (But is it really necessary? Possibly not).

This PR fixes a purely cosmetic issue and provides no additional functionality.

Before	After

kirb commented 3 years ago

I do need to revisit this, most importantly (to me) the Done button dismisses the entire Settings popup rather than just the browser. This PR will at least fix one thing, so thanks! Will merge in a bit.

By any chance, does also ignoring the top safe area fix the address bar when scrolling? (I’ve noticed this in other apps that are definitely using Safari VC via UIKit and not SwiftUI so it must be an Apple bug, but it’s worth attempting to work around it).

chrisharper22 commented 3 years ago

I can continue looking into this, but it does seem that the address bar size issue may be a SwiftUI bug itself.

chrisharper22 commented 3 years ago

With 69c6f2c, I redid the implementation for the Safari controller, and it now presents itself in a modal rather than pushing the navigation stack. This solves the issue of the Done button closing the whole Settings stack. This also solves the issue of the address bar becoming too small while scrolling; which is done by disabling the feature by setting the Configuration—which is not ideal, but it does work for now.

https://user-images.githubusercontent.com/56522199/115123062-490f2f80-9f89-11eb-9bdf-4667817d15f4.mov

kirb commented 3 years ago

So sorry, this got closed because I merged the newterm-3 branch to master. Could you edit this PR to point to master, or if it won’t let you, create a new PR pointing to master? Thanks!