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

iPhone scrolling is painful #595

Closed
cormacrelf opened this issue Feb 7, 2020 · 23 comments
Closed

iPhone scrolling is painful #595

cormacrelf opened this issue Feb 7, 2020 · 23 comments
Labels
A-frontend Area: Web frontend E-medium Effort: This requires a fair amount of work P-medium Medium priority

Comments

@cormacrelf
Copy link

Recently, docs.rs gained a fixed header bar. To accommodate this, all other content on the page was moved into an overflow: auto div of some kind. This is a problem. For context, there are two kinds of scrolling used on websites in Safari on an iPhone:

  1. Inertial scrolling, keeps moving until simulated friction stops it after you lift your finger. Covers a lot of ground very quickly but allows precise scrolling at low speed. Also has some priority over child type 2 elements for determining which content to scroll.
  2. Locked panning, 1:1 with touch movement, stops dead when finger lifts.

Type 1 is used for entire pages where the document is larger than the viewport. Type 2 is used for overflowing content, e.g. wide code blocks clipped to viewport width, scrolling horizontally.

It used to be that docs.rs pages were bigger than the viewport, and scrolled with type 1. Now that the entire page is in an overflow: auto div, Type 2 is used on everything. This means:

  1. Scrolling is slow, because no inertia. Docs pages are very long!
  2. You can get stuck in code blocks that are viewport height or larger. Because they take priority on touch events for scroll, you actually cannot scroll the page while they take over the screen. All touches are interpreted as locked horizontal pans, no matter how vertically you move your finger. Fortunately there is a small white margin on the left that allows (type 2) page scrolling, so you can get out, but it’s pretty bad.
  3. You can’t use the “tap status bar to scroll to top” feature any more.

Together, this is very painful UI. I use docs.rs from my phone every day. Please use position: fixed for fixed position navigation bars, or don’t do them at all.

@jyn514
Copy link
Member

jyn514 commented Feb 7, 2020

Hmm, maybe we should have gone with #510 after all ... cc @GuillaumeGomez @Koxiaet

@cormacrelf
Copy link
Author

FWIW the standard practice for the link-offset-with-fixed-header issue in that thread is to have an explicit, invisible, empty anchor tag with a name (maybe an id attribute instead, I’d have to check), absolutely positioned, relative to the original heading, with top: -$header_height. Navigation then scrolls to the invisible anchor, leaving it under the bar and the actual header visible. No JS necessary. Just an extra anchor for each navigable link.

@cormacrelf
Copy link
Author

For an example, open the devtools on this header: https://aglc4.cormacrelf.net/csl#3.1-Generally

@jyn514
Copy link
Member

jyn514 commented Feb 7, 2020

@cormacrelf would you be willing to make a PR implementing those CSS changes? It sounds like you know what you're talking about 😆

@cormacrelf
Copy link
Author

@jyn514 It would also dig into generating the HTML — it might be a bit tricky if you’re just slapping rustdoc output in a template. would appreciate a few pointers on how you’d like links to be transformed, if there’s another option than adding it to rustdoc itself or reparsing the whole thing. The CSS is pretty easy after that.

@cormacrelf
Copy link
Author

(And if that turns out to be a significant blocker, I would request reverting the top bar change that introduced this issue as an interim measure, as usability has been impacted so much.)

@jyn514
Copy link
Member

jyn514 commented Feb 7, 2020

It would also dig into generating the HTML

That's unfortunate, we are indeed just slapping rustdoc output into a template. Maybe you could coordinate with @GuillaumeGomez to change how rustdoc outputs HTML?

@jyn514
Copy link
Member

jyn514 commented Feb 7, 2020

Hmm but that doesn't fix old docs. This is definitely a hard problem, I can make a PR reverting the other changes in the meantime.

@GuillaumeGomez
Copy link
Member

Reverting the changes would bring back the anchor issue. :-/

@Kestrer
Copy link
Contributor

Kestrer commented Feb 7, 2020

I think they mean reverting all they way back to before the top bar was fixed (position: fixed, that is).

@GuillaumeGomez
Copy link
Member

Oh. That'd fix all issues then. :)

@cormacrelf
Copy link
Author

Sometimes a bit of vanilla JS on document load to insert anchors is just the right kind of solution. I’ll have a look at doing it that way, without changing rustdoc / having to queue a rebuild of every crate.

@Zexbe
Copy link
Contributor

Zexbe commented Feb 8, 2020

Scrolling is working on my phone (iPhone 8+)
Tested it on iPad pro, it also seems to work.

I did notice that sometimes, it would pause for a second like it was loading, before scrolling started working.

Is it every iPhone, or just some iPhones?
Is it every crate or just some crates?
What happens on an android phone?

@imjasonmiller
Copy link

I ran it locally and -webkit-overflow-scrolling on enables momentum scrolling and makes it a lot less painful on iPhone.

div.container-rustdoc {
    text-align: left;
+    -webkit-overflow-scrolling: touch;
}

You'd still lose out on being able to go back up quickly by tapping the top bar, but I think it alleviates a lot without having to rewrite a large part of the layout right now.

I could submit a pull request for that if that would suffice, but perhaps I'm missing something @cormacrelf?

@Nemo157
Copy link
Member

Nemo157 commented Feb 13, 2020

This also appears to make scrolling very laggy in macOS Safari (Version 13.0.5 (14608.5.12)) and Safari Technology Preview (Release 100 (Safari 13.2, WebKit 14610.1.2.1) and whenever switching back to the tab there's a flash of a blue border around the rustdoc_body_wrapper, which disappears after a frame moving the entire content a couple of pixels.

I tested removing both fixed attributes and the wrapper's bottom and overflow-y, and that fixed both the scroll laggyness and the flash of a border.

@jyn514
Copy link
Member

jyn514 commented Feb 16, 2020

@cormacrelf we reverted the change that happened around the time you opened the issue. Can you confirm whether the problem is fixed?

@jyn514
Copy link
Member

jyn514 commented Feb 21, 2020

@cormacrelf @Nemo157 @imjasonmiller Could one of you please confirm whether #610 helped with this issue?

@imjasonmiller
Copy link

Sorry @jyn514, I didn't see your first reply! It doesn't seem to have helped me with this issue, but perhaps I'm doing something wrong? After visiting the documentation for serde, it still seems the three points raised by @cormacrelf apply to me.

  1. Scrolling is slow, because no inertia.
  2. You can get stuck in code blocks that are viewport height or larger.
  3. You can’t use the “tap status bar to scroll to top” feature

@jyn514
Copy link
Member

jyn514 commented Feb 21, 2020

Ok, this bug must have existed for a while then since the CSS isn't updated all that often. I guess that means we can merge #611 (with apologies to @Zexbe for reverting their change).

If anyone wants to work on fixing this I'm happy to mentor but without an iPhone or Safari I can't really test any changes.

@jyn514
Copy link
Member

jyn514 commented Feb 22, 2020

I'm on the linked page with a friend's iPhone and I can't replicate any of the mentioned issues. This is an iPhone 8 with iOS 13. Can you post your version of iOS?

@imjasonmiller
Copy link

@jyn514 it's an older iPhone 5S, so that's iOS 12 still.

@rudedogg
Copy link

Having this issue on an iPhone 6 also, so iOS 12.

@jyn514 jyn514 added A-frontend Area: Web frontend E-medium Effort: This requires a fair amount of work P-medium Medium priority labels Jul 6, 2020
@jyn514
Copy link
Member

jyn514 commented Jul 13, 2020

Should be fixed in #873.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-frontend Area: Web frontend E-medium Effort: This requires a fair amount of work P-medium Medium priority
Projects
None yet
Development

No branches or pull requests

8 participants