WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 6 years ago

#29979 closed defect (bug) (fixed)

Twenty Fifteen: Long sidebar produces double-scrollbars

Reported by: kraftbj Owned by: iandstewart
Milestone: 4.1 Priority: normal
Severity: blocker Version: 4.1
Component: Bundled Theme Keywords: has-patch needs-testing
Focuses: ui, javascript Cc:

Description

The double scrollbar effect with long sidebars feels odd

https://cldup.com/4KDhUPFMAI.png
as seen currently on brandonkraft.com with a ~1600px by 700px viewport

The scrollbar is being added by .sidebar { overflow-y: auto; } on line 3808. Without it, it prevents scrolling.

Attachments (12)

29979.patch (472 bytes) - added by Jayjdk 7 years ago.
29979.2.patch (3.2 KB) - added by iseulde 7 years ago.
29979.3.patch (3.2 KB) - added by iseulde 7 years ago.
29979.4.patch (3.4 KB) - added by iseulde 7 years ago.
29979.5.diff (3.5 KB) - added by iamtakashi 7 years ago.
twenty-fifteen.php (751 bytes) - added by MikeHansenMe 7 years ago.
twenty-fifteen.2.php (949 bytes) - added by MikeHansenMe 7 years ago.
A few small changes. Should reach the bottom now of a long scrollbar.
twenty-fifteen.3.php (977 bytes) - added by japh 7 years ago.
twenty-fifteen.4.php (1.0 KB) - added by MikeHansenMe 7 years ago.
add scroll if content is smaller than window
twenty-fifteen.5.php (1.2 KB) - added by MikeHansenMe 7 years ago.
add scroll to small content by setting content size = sidebar
29979.6.diff (1.9 KB) - added by MikeHansenMe 7 years ago.
patch instead of plugin and reuse of variables
29979.7.diff (2.7 KB) - added by iamtakashi 7 years ago.
Fixiing the sidebar first and make it scroll if it's taller than the document. Props @mattwiebe

Download all attachments as: .zip

Change History (57)

#1 follow-up: @iseulde
7 years ago

Maybe we can do something similar to the side meta boxes in the post.php screen. I.e. scroll the sidebar with the content and keep its position fixed.

#2 @Jayjdk
7 years ago

  • Keywords has-patch added

I agree that it's very odd. There's no indication that the sidebar is scrollable and might be longer than the content.

I don't know what's the preferred solution is here but I've attached a patch that removes the fixed sidebar.

I tried experimenting with position: stickybut that didn't feel right either.

@Jayjdk
7 years ago

#3 in reply to: ↑ 1 @celloexpressions
7 years ago

  • Focuses ui added

Replying to avryl:

Maybe we can do something similar to the side meta boxes in the post.php screen. I.e. scroll the sidebar with the content and keep its position fixed.

I was about to suggest that too. Was proposed for Twenty Fourteen and not kept, but I think it's the best solution for the design of Twenty Fifteen.

#4 @SergeyBiryukov
7 years ago

  • Milestone changed from Awaiting Review to 4.1

The double scrollbar seems weird to me too, was the first thing that caught my eye.

#5 @Jayjdk
7 years ago

Here's how it behaves with position: sticky: https://cloudup.com/c6NAIi8pJOi

#6 @iseulde
7 years ago

It needs to scroll as soon as you start scrolling the content, and pin as soon as you hit the bottom of the sidebar, then unpin when you start scrolling up and pin as soon as you hit the top of the sidebar. We can take we did for editor scrolling. I'll take a look in a few hours.

#7 @kraftbj
7 years ago

I concur with avryl - the position:sticky is better, but the lack of scroll on the initial down scroll would practically result in the lower sidebar content being hidden too often.

#9 follow-up: @Otto42
7 years ago

Agreed. Possibly we might have an option in the customizer to move the sidebar to the right? More traditional. This would eliminate the separate scrollbar, which is frankly weird and immediately off-putting.

#10 in reply to: ↑ 9 @helen
7 years ago

Replying to Otto42:

Agreed. Possibly we might have an option in the customizer to move the sidebar to the right? More traditional. This would eliminate the separate scrollbar, which is frankly weird and immediately off-putting.

I think you'd still have two scrollbars, just next to each other instead :)

Would be cool to do the same thing as we do with the side metaboxes in the admin (which is a technique I've started to notice on other sites as well). Was also the first thing I noticed.

#11 @celloexpressions
7 years ago

  • Keywords needs-patch added; has-patch removed

See #24882 for the identical proposal for Twenty Fourteen. Might be revisiting this there too if we change it in Twenty Fifteen; I'm very much for it in both cases.

@iseulde
7 years ago

#12 @iseulde
7 years ago

  • Focuses javascript added
  • Keywords has-patch needs-testing added; needs-patch removed
  • Type changed from defect (bug) to enhancement

@iseulde
7 years ago

#13 @iseulde
7 years ago

has-better-patch

#14 @helen
7 years ago

Sidebar jumps down if it doesn't need to scroll with that patch. Worked well otherwise in my testing of trunk and a child theme, though.

@iseulde
7 years ago

#15 follow-up: @iseulde
7 years ago

Right, forgot about that case. :) It would jump if the sidebar is smaller than the window height.

#16 @kraftbj
7 years ago

Looking at 29979.4, I'm still seeing the scrollbars on FF and Chrome (OS X). On Safari, they're not there, but the screen is "flashy". Note: I didn't test on Safari without the patch.

Notice the top and bottom of the sidebar when scrolling: https://cloudup.com/ckArBv8fueZ

#17 @mor10
7 years ago

There are some novel solutions to the scrollbar issue that have been applied to other sites. Time.com restricts the scrollbar to just the scrollable area. This could be an option here: Lock the site title and description + possibly a sidebar menu and make the widgetized area independently scrollable.

To me the question is what the function of the sidebar is currently. If the function is as the place to house the site title and description then this information needs to stay put when the page is scrolled. If the function is as a place to house an endless list of widgets then the area needs to scroll with the overall content on the page to avoid bizarre behavior on touch screen devices. Right now, at least from my perspective, there is no clear answer to this question.

From a mobile perspective the current approach of having widgets comingled with the menu and site title/description is sub optimal. Imagine a site with 10 widgets. On your mobile device when you hit the hamburger in the top-right corner you get a drop-down area with the menu + all 10 widgets -> epic scrolling with questionable practical applications.

I guess to me at least the idea of allowing for widgets in the sidebar is in conflict with the assumed design and use case scenarios. If widgets are to be added they must be handled as separate entities and moved out of the contexts of both the sidebar as a whole (to solve the scrolling issue) and drop-down on smaller screens.

#18 in reply to: ↑ 15 ; follow-up: @azaozz
7 years ago

Replying to avryl:

Right, we can pretty much reuse the JS for the admin menu (added in [29835] and [29898]). However the sidebar will need to "work" when no JS and in weird browsers too. Perhaps better to leave the original CSS as-is, then override it with a class that is added from JS.

#19 in reply to: ↑ 18 @iseulde
7 years ago

Replying to azaozz:

Right, we can pretty much reuse the JS for the admin menu (added in [29835] and [29898]). However the sidebar will need to "work" when no JS and in weird browsers too. Perhaps better to leave the original CSS as-is, then override it with a class that is added from JS.

I set the position to absolute, so without JS it will just stay in its place (like you would do traditionally).
Which weird browsers? iOS? :)

#20 @azaozz
7 years ago

Well iOS, old Android, old IE, old ... whatever. Perhaps specifically disable for iPads if the onscreen keyboard interferes, etc.

#21 @iandstewart
7 years ago

  • Severity changed from normal to blocker

#22 follow-up: @mor10
7 years ago

To find a workable solution to this the original designer should be consulted. There is a clear conflict between the idea of a fixed sidebar with site title + menu and the idea of adding an unlimited list of widgets. This also causes serious problems on mobile as explained in my previous comment.

It would be useful to know what the original intent was with the sidebar and whether this can help lead a solution.

#23 in reply to: ↑ 22 ; follow-up: @iamtakashi
7 years ago

Replying to mor10:

It would be useful to know what the original intent was with the sidebar and whether this can help lead a solution.

It's a simple idea. If the sidebar is shorter than window, fix the position because that gives you better navigation experience. However if a user choose to put many widgets and to have a long sidebar, it obviously needs to be scrollable so that viewers can see all widgets.

#24 in reply to: ↑ 23 @mor10
7 years ago

Replying to iamtakashi:

That would mean tracking viewport height and conditionally fixing the sidebar based on whether the height is taller than the total area taken up by title + menu + widgets. I'm concerned about how that would work on smaller screens like tablets in horizontal mode.

Replying to mor10:

It would be useful to know what the original intent was with the sidebar and whether this can help lead a solution.

It's a simple idea. If the sidebar is shorter than window, fix the position because that gives you better navigation experience. However if a user choose to put many widgets and to have a long sidebar, it obviously needs to be scrollable so that viewers can see all widgets.

This ticket was mentioned in IRC in #wordpress-dev by iandstewart. View the logs.


7 years ago

#26 @iamtakashi
7 years ago

I've tried the patch 29979.4.patch. I like the sidebar never gets out of site like the admin.

Here are the issues I've spotted with the patch.

  • When we have a shorter sidebar, there is an inline style position: fixed for .sidebar, but that's causing an issue when a screen is resized to < 955px. https://i.cloudup.com/6zKGJhNWIG.png
  • The biggest issue I see is that on iPad landscape orientation. I wish we can just simply disable this fix but we don't have a relaiable way to detect touch device for front-end, do we?

29979.4.patch is now out of sync so here is the updated patch for easier testing.

@iamtakashi
7 years ago

#27 @MikeHansenMe
7 years ago

The file I just attached is a plugin (however it could easily be moved to the theme). Automatically scrolls the sidebar at a ratio when the content is scrolled. When the scrollbar is at the top both divs are at the top and when the scroll is at the bottom the divs are at the bottom. This adds a kinda interesting effect while scrolling.

@MikeHansenMe
7 years ago

A few small changes. Should reach the bottom now of a long scrollbar.

#28 @japh
7 years ago

I quite like @MikeHansenMe's solution here, though I was getting a weird bounce when scrolling beyond the top of the document (Mac, Chrome). So I added a 0 top if the content position shifted into negative digits, which has fixed it for me.

@japh
7 years ago

#29 @iamtakashi
7 years ago

@MikeHansenMe and @japh, Thanks for the suggestion! This is interesting and much simpler solution. With this approach, I didn't see the issues I saw in the previous suggestion. I really like the simplicity of it.

Another great thing with this approach is that the main content never go off the screen even when a sidebar is a lot longer than the main content. Also it works good on iPad landscape so far.

The only thing is that when we we're on a short post where there is no scroll available, there is no way currently to get a bottom of a long sidebar. Any idea to solve this issue?

@MikeHansenMe
7 years ago

add scroll if content is smaller than window

#30 @MikeHansenMe
7 years ago

twenty-fifteen.4.php does not seem to work as I expected.. It is adding the scrollbar when the sidebar is larger than the content. One other thing we will need to add is checking that the screen width is greater than the mobile breakpoint. So that we are not doing anything on mobile.

@MikeHansenMe
7 years ago

add scroll to small content by setting content size = sidebar

@MikeHansenMe
7 years ago

patch instead of plugin and reuse of variables

#31 @iandstewart
7 years ago

29979.5.diff feels most natural to me even though 29979.6.diff is pretty cool. It best mimics what I imagine to be expected behaviour when one scrolls.

Can we tackle these issues that @iamtakashi has highlighted with this approach?

  • When we have a shorter sidebar, there is an inline style position: fixed for .sidebar, but that's causing an issue when a screen is resized to < 955px.
  • On Safari (Mac), when we have a longer sidebar, after scrolling up a few times, the top part of the sidebar gets hidden. https://cloudup.com/cChe93pk860
  • The biggest issue I see is that on iPad landscape orientation. I wish we can just simply disable this fix but we don't have a reliable way to detect touch device for front-end, do we?

#32 @iandstewart
7 years ago

The biggest issue I see is that on iPad landscape orientation. I wish we can just simply disable this fix but we don't have a reliable way to detect touch device for front-end, do we?

Instead of detecting iPad … no fixed sidebar *ever* on mobile? If it's mobile, just scroll with content always?

#33 follow-up: @iseulde
7 years ago

Yes, we can use wp_is_mobile(). I'll take a look this evening.

#34 in reply to: ↑ 33 ; follow-up: @Monika
7 years ago

Replying to avryl:

Yes, we can use wp_is_mobile(). I'll take a look this evening.

if you are using wp_mobile, the solution will break if someone is using a caching system,
because wp_mobile is no js solution.

(my first track reply :-))

#35 @azaozz
7 years ago

29979.6.diff feels a bit "unnatural" at first (two page areas scrolling at different speeds?) but can get used to it :)

Will need to handle couple more edge cases:

  • If the post is very short and the sidebar is quite long, the JS "scrolling" of the sidebar is too accelerated/too fast and is hard to see what is in there or stop at a particular place. If scrolling with a mouse wheel, it just "jumps" which is disorienting.
  • The opposite case, shorter sidebar and very long content, the sidebar scrolls too slow. Need to do a lot of scrolling to see the bottom.

If this is implemented a TODO would be:

  • The default CSS for the sidebar should be position: static so the page height is set properly with one scrollbar. This will be used for small screens/mobile and when no-js.
  • The scroll ratio in scroll.js should have min and max limits, something like 0.5 to 2 seems appropriate.
  • In case the sidebar height is less than the viewport height we should set it to position: fixed (best done by adding/removing a class on #sidebar or body). This should be hooked in window resizing.
Last edited 7 years ago by azaozz (previous) (diff)

#36 in reply to: ↑ 34 @azaozz
7 years ago

Replying to Monika:

if you are using wp_mobile, the solution will break if someone is using a caching system...

Right. We can detect the device from JS and/or disable it in narrow windows.

#37 @mor10
7 years ago

Mobile-first is the way to go here: CSS and JS should be set up for the smallest screen and triggered only when the screen grows large enough to warrant its inclusion. The sidebar scrolling solution is only relevant on wider screens and should not require to be disabled on smaller ones. The issue isn't one of mobile device vs. desktop device; it's one of narrow vs. wide screen. The wp_is_mobile() hook looks for mobile devices but excludes things like narrow windows, snapped windows in Windows 8, and so on.

#38 @mor10
7 years ago

Tracking screen width and triggering custom JS to kick in when the screen is wide enough could be done in an understandable way (from the viewpoint of the end-user who may want to tamper with this in a child theme) using a solution like enquire.js or similar.

#39 @iandstewart
7 years ago

  • Type changed from enhancement to defect (bug)

@iamtakashi
7 years ago

Fixiing the sidebar first and make it scroll if it's taller than the document. Props @mattwiebe

#40 @iamtakashi
7 years ago

The patch above is not as interesting as previous suggestions but simplest direction so far.

It detects the sidebar height and then make it scroll if it's taller than the document, or stay fixed if it's shorter than the document.

#41 @iandstewart
7 years ago

The patch above is not as interesting as previous suggestions but simplest direction so far.

Uninteresting is a feature. Looks fit t'commit. Blocker begone.

#42 @iandstewart
7 years ago

  • Owner set to iandstewart
  • Resolution set to fixed
  • Status changed from new to closed

In 30025:

Twenty Fifteen: If the sidebar is taller than the viewport scroll it with the content, if it's shorter keep it fixed.

Props mattwiebe, iamtakashi, avryl, fixes #29979.

#43 @celloexpressions
7 years ago

I think we should reconsider this; specifically, reconsider doing what 29979.5.diff tries to do, mimicking the admin menu behavior. When the sidebar is slightly taller than the content window (especially notable on archive views), you end up losing the sidebar pretty quickly. This is pretty awkward even on http://twentyfifteendemo.wordpress.com/. That was the first feedback I got from several people seeing the theme for the first time.

This ticket was mentioned in Slack in #core by johnbillion. View the logs.


6 years ago

This ticket was mentioned in Slack in #core by iandstewart. View the logs.


6 years ago

Note: See TracTickets for help on using tickets.