Scroll the sidebar when it's collapsed

Working on this now!

I'm struggling to find the solution for this one. Adding overflow-y: scroll; to the sidebar when it's collapsed means the tooltips and flyouts are hidden. I am going to unassign myself for now in case someone else can pick this up.

Pushed a commit for flyout menu to appear when collapsed but still need to address the tooltips that aren't displayed correctly

Should work now

Those changes look great @hooroomoo, thanks so much for taking this! Just approved the MR, I think it's safe to merge :)

i'm having trouble finding a solution for firefox so if anyone else wants to jump on this please feel free

I'll take another crack at this!

I believe the Firefox issue is resolved now. Ready for another review!

Tested on chrome, firefox, and safari and it looks good to me. Pushed a small fix to Safari because the flyouts and tooltips were getting clipped when there was overflow.

I will point out that the scrollable elements don't include the Drupal icon and Bookmarks after the Safari fix since the fix was to add overflow: y to the element which excludes the Drupal icon and Bookmarks.

Thanks for catching that safari issue @hooroomoo!

I will point out that the scrollable elements don't include the Drupal icon and Bookmarks after the Safari fix since the fix was to add overflow: y to the element which excludes the Drupal icon and Bookmarks.

I'm not sure if this is the right solution, as it's not visually apparent what area can be scrolled. Shadows would help but we might want some input for @ckrina if we go this route.

The overarching issue here is that "setting one axis to visible (the default) while setting the other to a different value results in visible behaving as auto" (source: MDN). So some browsers are allowing the overflow: visible for the flyouts and tooltips but others (eg. safari) are not.

I wonder if we need to look into taking the markup for the flyouts out of the sidebar altogether but that could introduce a whole other set of issues, likely including accessibility issues since the DOM order wouldn't be logical.

@ckrina or @mherchel, I'd love to get your thoughts on this.

For some reason tugboat isn't working on this issue so I've uploaded a quick recording of it's current state.

I've taken a bit of a look at this and have a couple thoughts:

1. As noted by the comments above, it's not possible to let content overflow:visible in one axis, and do a overflow:auto in another. To work around this, we'll need to dynamically append the flyout submenus to the end of the DOM (before </body>), and absolutely position them via FloatingUI. This is possible, but will take a bit of work, and refactoring.
2. My main question is if this fix is needed for testing. At some point the JavaScript will need to be refactored into more specific functions, better usage of Drupal Behaviors, etc. Because of the nature of the rapid development and changes, it's looking somewhat jQuery-ish, with everything in one big behavior. My thought is that if this doesn't affect the testing, we should postpone this until we can refactor the JS. In the meantime, we should concentrate on items that directly affect the tests including getting this to work on mobile.

Thoughts?

Gonna do some js/css cleanup.

@finnsky - Glad I checked this. I was in the process of doing a lot of the same. I'm about to commit the JS with updated documentation.

I'm going to start working on this again, starting with a new branch and implementing @mherchel's suggestion:

we'll need to dynamically append the flyout submenus to the end of the DOM (before </body>), and absolutely position them via FloatingUI

@claireristow btw it may work with FloatingUI elements in fixed(not absolute) position.

Hey @finnsky, thank you so much for the suggestion!

I just did a quick test on my local and it worked! There's even an explanation about our exact problem in the floatingUI docs with suggestions to used fixed positioning instead. This method will likely save a lot of js work and future headaches! I didn't see your comment until after I pushed up some WIP stuff on my new `3380251-scroll-refactor-flyouts` branch but I'm going to start a third branch with the "fixed" method.

Oops, false alarm. Sadly the "fixed" method doesn't work in all browsers. Darn Safari! I'm going to keep working on my refactor flyouts branch.

I have a working example up at the tugboat link with two bugs that I need to spend more time on. Just noting them here for myself or in case others are reviewing and see these:

1. When the sidebar is collapsed, if you slowly mouseover an icon from the right, it will flicker open and then closed.
2. The "autoExpandToActiveMenuItem" function is being called before a "closeAllSubmenus" call which closes the active trail in the flyout. I think closeAllSubmenus is being used in too many places so some re-factoring may be needed there.

I haven't yet started on the keyboard nav and focus handling now that we've messed with the DOM structure. I would like some guidance an what is expected for this. Here are my questions:

1. What is expected when you tab to a sidebar icon (in the collapsed state). Should the flyouts/tooltips show on focus, on "enter", or other. Maybe different experiences between flyouts and tooltips since one is a button and one is a link?
2. Should flyouts have a focus trap?

Should flyouts have a focus trap?

No. The point of a focus trap is to disable anything from gaining focus while not visible (if it's behind a flyout or modal). We might need to add logic to have the flyouts close on blur (focusout), but that can be outside of this.