Change 970463 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] [WIP] useFloatingMenu: Add clipping and flipping

https://gerrit.wikimedia.org/r/970463

Change 970904 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] Menu: Update border-radius when Menu flips

https://gerrit.wikimedia.org/r/970904

The patches for this task (and T346696) can be tested here.

The behavior implemented in these patches is as follows:

• Clipping: If the menu doesn't fit between the triggering element (text input or select handle) and the bottom of the viewport, its height is reduced so that it fits, and it's made scrollable
  • If there is a footer element, it's sticky: the rest of the menu becomes scrollable, but the footer stays at the bottom no matter how the menu is scrolled. This is the same behavior as when the menu is scrollable because the number of items exceeds the visibleItemLimit prop.
  • We keep a small buffer (called "padding" in floating-ui) between the bottom of the menu and the bottom of the viewport. This makes it clearer visually that the menu is cut off above the edge of the screen; if we didn't have this buffer space, and the bottom border of the menu was right at the bottom of the viewport, it would look like the menu was overflowing past the edge of the screen.
  • In my patch this padding is currently set to 8 pixels, but I made that number up, and I defer to @bmartinezcalvo as to what the right number would be
• Flipping: If the triggering element is close to the bottom of the viewport, so that the menu would be too small if it was below the triggering element (opened downwards), it will flip to be above the triggering element (open upwards)
  • In my patch the definition of "too small" is 100 pixels: if positioning the menu below the triggering element would make it less than 100px tall, it will be positioned above instead. I also made this number up (the floating-ui documentation suggested 50px, but I thought that was too small) and I defer to @bmartinezcalvo for what this number should be as well
  • If there also isn't 100px of space for the menu above the triggering element, the behavior is a little strange: the menu will open upwards, and will run off the screen, because it refuses to be 100px tall. We could probably fix this, but I haven't focused on this much yet, because the screen would have to be pretty small for this to matter.
  • When the menu is flipped (opens upwards), the rounded corners on the menu and the triggering element are adjusted. When the menu opens downwards, the menu's bottom corners and the triggering element's top corners are rounded, but the menu's top corners and the triggering element's bottom corners are straight. When the menu opens upwards, this is reversed.
  • When the menu is flipped, the box-shadow on the menu is not adjusted currently. There's still a box-shadow at the bottom of the menu, but it doesn't seem to be visible -- I think it probably disappears beneath the triggering element. How should we handle this? Should we remove the box-shadow? Should we move the shadow to the top of the menu?

In T346689#9300310, @Catrope wrote:

• In my patch this padding is currently set to 8 pixels, but I made that number up, and I defer to @bmartinezcalvo as to what the right number would be

@Catrope I've reviewed the patch and it looks good. But I would expand the padding to 16px instead since with 8px padding the menu with the shadow seems to be too close to the edge of the screen. I think adding a bit more space between the menu and the edge will work better.

• Flipping: If the triggering element is close to the bottom of the viewport, so that the menu would be too small if it was below the triggering element (opened downwards), it will flip to be above the triggering element (open upwards)
  • In my patch the definition of "too small" is 100 pixels: if positioning the menu below the triggering element would make it less than 100px tall, it will be positioned above instead. I also made this number up (the floating-ui documentation suggested 50px, but I thought that was too small) and I defer to @bmartinezcalvo for what this number should be as well

@Catrope What if we expand tthis minimum menu size from 100px to 128px using a number from our scale?

@bmartinezcalvo I've updated the patch to use 16px and 128px.

Also, @AnneT left this comment in Gerrit:

I think this is a major improvement, but I do have concerns about the flipping behavior when it occurs after the menu has already opened. I think this experience is a bit jarring:

• Open a menu that has plenty of room to open down
• Scroll up. The menu will start to clip, then will flip right before it disappears (since it closes when the triggering element leaves the viewport)

I think this is why OOUI menus open in one direction and stay here.

That said, flipping only initially is not supported out of the box by FloatingUI, and would be tedious to implement and maintain. So I suggest we pose this question to the design team: is the behavior added in this patch acceptable? If not, is it a blocker, or could we improve the behavior over time?

@bmartinezcalvo what do you think? I can try to see how hard it would be to only flip the menu when it opens and then keep it in the same orientation, but we should account for the possibility that this might not be feasible. Alternatively, I wonder if the increased padding and the increased minimum height make this issue a bit less bad?

thank for sharing @Catrope! from an early test, i noticed 2 small things that we might want to consider:

1. when the menu flips above it covers/hides the field label possibly loosing context
   
2. when the menu opens, if the clipping is roughly N menu items high it might not be immediately clear (at least on iOS and macOS) that you can scroll the results.
   

i'll also share this task with the Abstract Wikipedia team given that we heavily use selects and lookups.

two other small things (maybe regressions?):

• on mobile it seems that the border radius of the lookup is larger compared to the lookup v1.0.1
• when the menu is open the bottom corners radius (or top corners if the menu is flipped) are not set to 0

patch demo

v1.0.1

In T346689#9318718, @AAlhazwani-WMF wrote:

thank for sharing @Catrope! from an early test, i noticed 2 small things that we might want to consider:

1. when the menu flips above it covers/hides the field label possibly loosing context
   

That's an interesting trade-off. If there was ever a reason why we might want to allow components to disable flipping, this could be one. @bmartinezcalvo what do you think the best approach is here? Should the menu flip, even if that obscures the label for the Field it's in?

2. when the menu opens, if the clipping is roughly N menu items high it might not be immediately clear (at least on iOS and macOS) that you can scroll the results.
   

Unfortunately I don't think there's much we can do about this. I don't believe Mac OS / iOS allow us to force a scrollbar to be shown. (If you know of a way to do this in CSS please let me know; I only know how to force the scrollbar to be hidden.)

In T346689#9319187, @AAlhazwani-WMF wrote:

two other small things (maybe regressions?):

• on mobile it seems that the border radius of the lookup is larger compared to the lookup v1.0.1
• when the menu is open the bottom corners radius (or top corners if the menu is flipped) are not set to 0

patch demo

I can't reproduce this, neither on my laptop by simulating an iPhone, nor by going to the demo on my Android phone. Which URL are you seeing that at? I'm looking at https://970904--wikimedia-codex.netlify.app/components/demos/lookup.html#with-custom-menu-item-display-1, and I see this:

In T346689#9321447, @Catrope wrote:

Unfortunately I don't think there's much we can do about this. I don't believe Mac OS / iOS allow us to force a scrollbar to be shown. (If you know of a way to do this in CSS please let me know; I only know how to force the scrollbar to be hidden.)

yeah, i don't think it's possible to force display scrollbars, at least in a non-hacky way. the only thing that comes up to my mind is to use

::-webkit-scrollbar {
  -webkit-appearance: none;
  width: 7px;
}

::-webkit-scrollbar-thumb {
  -webkit-box-shadow: 0 0 1px rgba(255,255,255,.5);
  border-radius: 7px;
  background-color: rgba(0,0,0,.5);
}

but from a quick test it seems not to work on iOS, so yeah, dead end :) what i was wondering instead is if we might want to consider alternative visual solutions, these ideas are just shared as food for thoughts:

• a bottom gradient, that hides as soon as you scroll the menu
  
• or a signifier, that hides as soon as you scroll the menu
  

I can't reproduce this, neither on my laptop by simulating an iPhone, nor by going to the demo on my Android phone. Which URL are you seeing that at? I'm looking at https://970904--wikimedia-codex.netlify.app/components/demos/lookup.html#with-custom-menu-item-display-1, and I see this:

i'm able to reproduce both issues (the increased border radius, and the missing border radius reset when the menu is open) if i visit the link that you shared on an iPhone SE 2020, iOS 17.1.1, both Safari and Firefox for iOS

and also when i open the same link in the iOS simulator, iPhone SE 2023, iOS 16.4, Safari

In T346689#9322243, @AAlhazwani-WMF wrote:

i'm able to reproduce both issues (the increased border radius, and the missing border radius reset when the menu is open) if i visit the link that you shared on an iPhone SE 2020, iOS 17.1.1, both Safari and Firefox for iOS

and also when i open the same link in the iOS simulator, iPhone SE 2023, iOS 16.4, Safari

Thanks for sharing this Amin! The key piece of information was that this only happens on iPhones. This allowed @lwatson to track this down to an iOS Safari-specific UA style rule and fix it. The demo should now be updated with her fix.

In T346689#9321447, @Catrope wrote:

In T346689#9318718, @AAlhazwani-WMF wrote:

thank for sharing @Catrope! from an early test, i noticed 2 small things that we might want to consider:

1. when the menu flips above it covers/hides the field label possibly loosing context
   

That's an interesting trade-off. If there was ever a reason why we might want to allow components to disable flipping, this could be one. @bmartinezcalvo what do you think the best approach is here? Should the menu flip, even if that obscures the label for the Field it's in?

@Catrope as a user, I prefer to lose the label's reference while selecting the menu item rather than scroll within a short menu trying to find all the menu items. It's true that the user will lose the field's context, but this will occur after the user has read the label and clicks/types in the input field when the menu is displayed, so the user will have read the label before. Also, the label will be hidden just while the user is typing in the input or selecting the menu item, in which the focus should be on the menu.

A possible solution to reinforce the field's context for the user could be using the placeholder prop within the input (e.g. "Type your country" so that the user can easily identify that this field is for "Country").

Anyway, if we want to collect more feedback from real users, we could conduct user testing to solve this specific and other potential questions.

Change 970463 merged by jenkins-bot:

[design/codex@main] useFloatingMenu: Add clipping and flipping

https://gerrit.wikimedia.org/r/970463

Change 970904 merged by jenkins-bot:

[design/codex@main] Menu: Update border-radius when Menu flips

https://gerrit.wikimedia.org/r/970904

@Catrope it looks better now with the 16px padding between the edge of the screen and the menu, and also with the minimum size menu as 128px. However, when checking the demo I've realized that there is a point in the scroll where the 16px becomes smaller. Is it possible to fix this?

Also, there is 1px separation between the input and the menu when the menu is displayed below, while that 1px disappears when the menu is displayed on top.

Change 979184 had a related patch set uploaded (by Catrope; author: Catrope):

[design/codex@main] useFloatingMenu: Also apply padding to flip()

https://gerrit.wikimedia.org/r/979184

In T346689#9370906, @bmartinezcalvo wrote:

@Catrope it looks better now with the 16px padding between the edge of the screen and the menu, and also with the minimum size menu as 128px. However, when checking the demo I've realized that there is a point in the scroll where the 16px becomes smaller. Is it possible to fix this?

Good catch! This was happening because I had set the padding for the size middleware (which makes the menu smaller) but not for the flip middleware (which flips it). I've uploaded a patch that fixes this (demo).

Also, there is 1px separation between the input and the menu when the menu is displayed below, while that 1px disappears when the menu is displayed on top.

I think the 1px separation is itself a bug. It only happens in the Lookup examples, not in the Menu examples, and it doesn't happen at all zoom levels. I think it probably happens because of rounding errors in the browser: the input is 32.84px tall, and the menu is positioned 33px below the top of the input. This "should" result in a consistent 0.16px gap, which appears to be rounded down to 0 most of the time but not always. This is caused by T341224, and in fact Anne flagged this exact same bug on that task in September.

In T346689#9373804, @Catrope wrote:

In T346689#9370906, @bmartinezcalvo wrote:

@Catrope it looks better now with the 16px padding between the edge of the screen and the menu, and also with the minimum size menu as 128px. However, when checking the demo I've realized that there is a point in the scroll where the 16px becomes smaller. Is it possible to fix this?

Good catch! This was happening because I had set the padding for the size middleware (which makes the menu smaller) but not for the flip middleware (which flips it). I've uploaded a patch that fixes this (demo).

@Catrope thank you, this is now fixed in Chrome and Safari where the min-height is 128px and the padding below is 16px. But in Firefox the menu min-height is more than 128px. Could you check this?

Also, there is 1px separation between the input and the menu when the menu is displayed below, while that 1px disappears when the menu is displayed on top.

I think the 1px separation is itself a bug. It only happens in the Lookup examples, not in the Menu examples, and it doesn't happen at all zoom levels. I think it probably happens because of rounding errors in the browser: the input is 32.84px tall, and the menu is positioned 33px below the top of the input. This "should" result in a consistent 0.16px gap, which appears to be rounded down to 0 most of the time but not always. This is caused by T341224, and in fact Anne flagged this exact same bug on that task in September.

Ok, thank you for the context. In case you need to check it, in this page demo page the problem with the 1px below the input is just happening in the demo With custom menu item display.

Change 979184 merged by jenkins-bot:

[design/codex@main] useFloatingMenu: Also apply padding to flip()

https://gerrit.wikimedia.org/r/979184

Change 980902 had a related patch set uploaded (by Anne Tomasevich; author: Anne Tomasevich):

[mediawiki/core@master] Update Codex from v1.0.1 to v1.1.1

https://gerrit.wikimedia.org/r/980902

Test wiki created on Patch demo by ATomasevich (WMF) using patch(es) linked to this task:
https://patchdemo.wmflabs.org/wikis/d419984345/w

Change 980902 merged by jenkins-bot:

[mediawiki/core@master] Update Codex from v1.0.1 to v1.1.1

https://gerrit.wikimedia.org/r/980902

Ugh, the bug on Firefox is much worse than I thought: it alternates between top and bottom more or less every time you scroll:

This is probably an upstream bug in floating-ui, I'll try to come up with a minimal reproduction and submit it upstream.

@Catrope can you help me understand how to reproduce this?

In T346689#9390653, @AnneT wrote:

@Catrope can you help me understand how to reproduce this?

In Firefox, go to the Menu demos page, open a tall enough menu, and then very slowly scroll up (ideally using the trackpad on your laptop rather than using a mouse). On my computer, this causes the menu to flip repeatedly when it's in a certain height range.

Several people have failed to reproduce this in Firefox on Mac, so it may be an Ubuntu-specific issue.

I also tried this in Replay. Unfortunately it doesn't seem to work well on Ubuntu (or at least not on my laptop), the tab crashes every time I end a recording, and the recording isn't saved. However, I did notice that Replay's browser (which also appears to be Firefox-based) behaves a little differently, and more in line with what others have been seeing: instead of flipping up and down repeatedly, the menu flips before it clips. This is the same behavior that @bmartinezcalvo demonstrated in her video.

Given all the patches are merged, can we resolve this task and file a new task for the bug?

Test wiki on Patch demo by ATomasevich (WMF) using patch(es) linked to this task was deleted:

https://patchdemo.wmflabs.org/wikis/d419984345/w/