WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#30208 closed defect (bug) (fixed)

Twenty Fifteen: expanding a sub-menu when the sidebar is fixed needs to conditionally unfix the sidebar

Reported by: celloexpressions Owned by: iandstewart
Milestone: 4.1 Priority: normal
Severity: blocker Version: 4.1
Component: Bundled Theme Keywords: has-patch commit
Focuses: ui Cc:

Description

Decided it would be better not to reopen #29979 at this point due to its length, but the associated code needs to account for this case. If it's short enough that the sidebar is fixed, but you staart opening submenus, you are unable to scroll the sidebar. This can potentially make it impossible to access certain submenu items on certain screen sizes, which site owners will not necessarily realize since it may be okay on their screen sizes. Reproducible on http://twentyfifteendemo.wordpress.com/.

FWIW I'm still not a fan of this approach - going with something that also partially fixes the sidebar when it overflows would avoid this issue. If you're scrolled down the page when you run into this and the sidebar unfixes, it'll have to jump to the top, out of view.

Attachments (2)

30208.diff (1.4 KB) - added by mattwiebe 6 years ago.
30208.1.diff (1.4 KB) - added by mattwiebe 6 years ago.

Download all attachments as: .zip

Change History (14)

#1 @iandstewart
6 years ago

  • Milestone changed from Awaiting Review to 4.1

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


6 years ago

This ticket was mentioned in Slack in #core-themes by iamtakashi. View the logs.


6 years ago

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


6 years ago

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


6 years ago

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


6 years ago

#7 @iandstewart
6 years ago

  • Severity changed from normal to blocker

@mattwiebe
6 years ago

#8 @mattwiebe
6 years ago

  • Keywords has-patch added; needs-patch removed

30208.diff only introduces sidebar fixing logic if no submenus are present.

@mattwiebe
6 years ago

#9 @mattwiebe
6 years ago

30208.1.diff because code style :)

#10 @iandstewart
6 years ago

To bring out the comments from our discussion in Slack onto the ticket, there are likely better solutions than "just not fixing the sidebar when there is a menu with submenus" but this solution _is_ a solution. It works.

"It's less than ideal, because in my mind there is a JS solution that would be perfect. But it's a good chunk of JS work. And we're running out of time." — @mark

Committing @mattwiebe's solution and closing the ticket.

#11 @iandstewart
6 years ago

  • Keywords commit added

#12 @iandstewart
6 years ago

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

In 30324:

Twenty Fifteen: unfix the sidebar if we have a menu with submenus as there is a chance visitors might not be able to scroll down to see submenus in some situations.

Props mattwiebe, fixes #30208.

Note: See TracTickets for help on using tickets.