Make WordPress Core

Opened 10 years ago

Closed 10 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's profile celloexpressions Owned by: iandstewart's profile 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 10 years ago.
30208.1.diff (1.4 KB) - added by mattwiebe 10 years ago.

Download all attachments as: .zip

Change History (14)

#1 @iandstewart
10 years ago

  • Milestone changed from Awaiting Review to 4.1

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


10 years ago

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


10 years ago

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


10 years ago

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


10 years ago

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


10 years ago

#7 @iandstewart
10 years ago

  • Severity changed from normal to blocker

@mattwiebe
10 years ago

#8 @mattwiebe
10 years ago

  • Keywords has-patch added; needs-patch removed

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

@mattwiebe
10 years ago

#9 @mattwiebe
10 years ago

30208.1.diff because code style :)

#10 @iandstewart
10 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
10 years ago

  • Keywords commit added

#12 @iandstewart
10 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.