Skip to content

Conversation

@davidhatten
Copy link
Contributor

@davidhatten davidhatten commented Jan 19, 2026

This change combines a few behaviors to result in a better experience on mobile. This does affect desktop as well.

The P1/P2 selector was performing incorrectly at the right edge of the screen. This change forces the menuitems to behave correctly.

Current Mobile behavior:
image

Current mobile behavior (zoomed out):
image

New Mobile Portrait:
image

New Mobile Landscape:
image

Current Desktop behavior:
image

New Desktop behavior:
image

New Desktop 5, 7, and 9 card draws
image

@vercel
Copy link

vercel bot commented Jan 19, 2026

@davidhatten is attempting to deploy a commit to the noahm's projects Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Jan 22, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
ddr-tools Ready Ready Preview Jan 22, 2026 1:34am

Copy link
Owner

@noahm noahm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the OverlaysProvider? Yes, that's long overdue.

The rest of the style changes? I'm not so sure about, and I don't really see how they improve UX or the specific edge-of-screen issue for card actions and the 2nd layer menus.

Personally I'm not interested in fixing the card aspect ratio in this way, and prefer to have a more fluid design that allows them to fill different sized spaces. (This also affects how they can be presented on streams as something that must fit into the rest of the stream's visual layout.)

import "@blueprintjs/datetime/lib/css/blueprint-datetime.css";

import { FocusStyleManager } from "@blueprintjs/core";
import { OverlaysProvider } from "@blueprintjs/core";
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh... I didn't realize that I wasn't using this yet. That's a big oversight on my part. I think this is the main thing that improves the 2nd layer menus positioning within the screen?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately this had no noticeable effect but yeah the docs says it's necessary for overlays so 🤷

@davidhatten
Copy link
Contributor Author

davidhatten commented Jan 22, 2026

I don't really see how they improve UX or the specific edge-of-screen issue for card actions and the 2nd layer menus.

take a very close look at the first screenshot; the nested menu is open but the boundary logic is failing to flip the nested menu to the other side. On mobile you can sort of pinch to zoom out but you're fighting a view layer that disagrees with the opinion that the draw area is suddenly bigger and it will try to reset you back to default.
image

Here's the zoomed out view in the current mobile experience when trying to select a player. You have to actively zoom out to be able to see this once you open the menu
image

Heard on the aspect ratio fixing tho, i'll play with it a bit more and see what I can come up with

@noahm
Copy link
Owner

noahm commented Jan 24, 2026

take a very close look at the first screenshot;

yeah, I'm following the issue generally, and I agree it's sucky behavior that needs a fix. I just didn't expect these other changes to the card shape to actually be involved in fixing that, versus... whatever the OverlaysProvider does? I'll also poke around at this problem more when I get some time.

@noahm
Copy link
Owner

noahm commented Jan 24, 2026

@davidhatten On further inspection, I'm not actually sure how you arrived at a point where this appeared to resolve the issue for you. In my testing on this branch the exact same problematic behavior happens in both iOS safari and in "responsive mode" on the desktop versions of Chrome and Firefox. I'm glad you're bringing attention to this issue! 🙇

...but unfortunately I don't think we've found a real fix just yet. I'm a little stumped too, since the blueprint menus system has very deliberate behavior to keep items on screen. I'm pretty sure something else I'm doing with my app's layout is probably breaking that behavior, but I have no idea what.

@davidhatten
Copy link
Contributor Author

@davidhatten On further inspection, I'm not actually sure how you arrived at a point where this appeared to resolve the issue for you. In my testing on this branch the exact same problematic behavior happens in both iOS safari and in "responsive mode" on the desktop versions of Chrome and Firefox. I'm glad you're bringing attention to this issue! 🙇

...but unfortunately I don't think we've found a real fix just yet. I'm a little stumped too, since the blueprint menus system has very deliberate behavior to keep items on screen. I'm pretty sure something else I'm doing with my app's layout is probably breaking that behavior, but I have no idea what.

Oh no, it's bigger than I thought. What's "Responsive Mode"? I emulated an android device using chrome devtools
image

I wonder if there's a flexbox somewhere there shouldn't be. It doesn't look like the card draw tray itself is growing, but I haven't been able to find a useful element in this state yet
image

take a very close look at the first screenshot;

yeah, I'm following the issue generally, and I agree it's sucky behavior that needs a fix. I just didn't expect these other changes to the card shape to actually be involved in fixing that, versus... whatever the OverlaysProvider does? I'll also poke around at this problem more when I get some time.

Yeah I have no idea what the OverlayProvider actually does, but the docs said its necessary, especially for these menus. Maybe the animations improved after I added it? Hard to say and since it didn't immediately fix the problem or break anything, I moved on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants