Problem/Motivation

The flyouts are not currently centred to their parent, as shown in designs. They are flowing either from the top/down or top/up, depending on if the parent link is in the upper or lower half of the viewport.

Steps to reproduce

Observe the positioning of the "content" flyout compared to the user flyout. One flows down and one flows up.

screenshot of the content flyout

screenshot of the user flyout

Proposed resolution

As a general rule, position the flyouts in the centre of the parent link as seen in designs. Add some javascript to listen if the submenu will flow offscreen. If so, re-position the flyout so it's always visible.

Issue fork navigation-3376222

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

claireristow created an issue. See original summary.

claireristow’s picture

Issue summary: View changes
claireristow’s picture

Issue summary: View changes
claireristow’s picture

Issue summary: View changes
claireristow’s picture

@mherchel suggested using the Floating UI library which should be in core when the Splitbutton issue is merged. Since it's not in core yet, we can add the library to this module and use it for the placement of the flyout menus.

Utkarsh_33 made their first commit to this issue’s fork.

utkarsh_33’s picture

Assigned: Unassigned » utkarsh_33

utkarsh_33’s picture

Status: Active » Needs review
utkarsh_33’s picture

Assigned: utkarsh_33 » Unassigned
mherchel’s picture

Status: Needs review » Needs work

Thanks for the work on this. There are several things wrong.

  • JS console error while hovering floating-ui.dom.umd.js?v=10.1.1:157 Uncaught (in promise) TypeError: Cannot read properties of null (reading 'getBoundingClientRect')
  • Flyouts still aren't positioned properly. When I intentionally make my screen very short (to where it covers part of the configuration flyout), I expect the flyout to reposition so it is not obscured by the bottom of the viewport. Unfortunately this doesn't happen.
  • It's very difficult to move the mouse from the button that triggers the flyout to the actual flyout. Frequently the flyout will close very quickly.
  • The flyout still isn't vertically centered.
  • It also needs to be brought up to date with the main 1.x branch
utkarsh_33’s picture

I have addressed some of the feedbacks mentioned in #11.I think we need to handle flyout's position using some css.For now i have removed the offset used in the floating UI library which solves the problem of flyout closing quickly.

claireristow’s picture

StatusFileSize
new68.99 KB
new125.38 KB

Thanks for addressing that feedback @Utkarsh_33!

I just took a look and I'm seeing the following two bugs:

  1. The flyouts for items at the top and bottom of the screen are being cut off. They should be repositioned with the help of the js library so they're always visible.
  2. Some of the contents of the flyouts are overflowing from their containers

Screenshot - issue 1

Screenshot - issue 2

hooroomoo’s picture

Assigned: Unassigned » hooroomoo

working on this

hooroomoo’s picture

Assigned: hooroomoo » Unassigned
Issue summary: View changes
StatusFileSize
new85.76 KB
new96.71 KB

Flyout menus are vertically centered now and addressed the two bugs in comment #13.

utkarsh_33’s picture

Attaching the screen shots demonstrating that the arrow element is attached to the flyouts.

utkarsh_33’s picture

utkarsh_33’s picture

hooroomoo’s picture

Status: Needs work » Needs review
claireristow’s picture

Status: Needs review » Needs work
StatusFileSize
new134.9 KB

Hi @hooroomoo, thanks for your work on this. It's looking great!

I'm just seeing one more bug. When you hover over the user menu (see screenshot), there is some funky formatting and the flyout is showing in the middle of the screen.

Screenshot of user menu issue

hooroomoo’s picture

Status: Needs work » Needs review

#20 should be fixed now

mherchel’s picture

Status: Needs review » Reviewed & tested by the community

This is looking really good! Setting to RTBC because the goal of this is testing. A couple notes

  • RTL doesn't work. This can be fixed when we move to Drupal
  • If you have a short viewport, and activate a large hover flyout near the bottom of the screen it positions properly.... but, if you then expand a submenu, the menu will shift so that some menu items are obscured by the edge of the screen. Ideally, we'd have an event listener check for that and reposition only if necessary. That being said, this is an edge case, and we can fix it followup.

Excellent work. Than you!

mherchel’s picture

Setting credits

ckrina made their first commit to this issue’s fork.

ckrina’s picture

Status: Reviewed & tested by the community » Fixed

Merged this so the improvements can used on the testing and the missing parts and @todos can be worked in follow-ups with smaller MRs.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.