Dialogs of scrolled buttons don't align

[ayush80942]

Fixes a issue dropped in #code-todo-list https://discord.com/channels/731730685944922173/881073965047636018/1450985374292906097

Test Results -

Screen.Recording.2025-12-20.at.7.mp4

[timon-schelling]

!build

[github-actions]

📦 Build Complete for 87c0ef6
https://79803a58.graphite.pages.dev

[timon-schelling]

The bumping logic seems off.

[ayush80942]

Hi @timon-schelling,

Thanks for the review! I want to make sure I understand the issue correctly before making changes.

Regarding "The bumping logic seems off" - could you clarify what specific behavior you're seeing?

I'm wondering if you're referring to:

1. The dialog not being properly centred under its button when it opens? - It seems ok though
2. The dialog position when it would overflow the window edges?
3. The hover transfer behavior (when moving the mouse from one dropdown button to another)?

A quick description or additional screenshot/screen-recording would help me fix this correctly on the first try.

Thanks!

[timon-schelling]

The second one, the bumping logic still needs to apply but also take into account the scroll position.

[ayush80942]

Fixed the issue @timon-schelling

Now, the dialog stays visible completely till the button stays on the screen while scrolling.

[timon-schelling]

!build

[github-actions]

📦 Build Complete for 2fa4740
https://38b176e4.graphite.pages.dev

[timon-schelling]

Good work, dialog should be closed when the button is not on screen, and the marker should not move past the dialog (minus corner radius).

[ayush80942]

Fixed.

[timon-schelling]

!build

[github-actions]

📦 Build Complete for dd411a5
https://a69a31f8.graphite.pages.dev

[timon-schelling]

That solution leads to incorrect positions for dialogs originating from the side panel. see master behavior for comparison.

[ayush80942]

Apologies! Work fine now.

[timon-schelling]

!build

[github-actions]

📦 Build Complete for 1362c0b
https://b57bf18f.graphite.pages.dev

[timon-schelling]

Sorry for taking so long to take a look at the code again.
A have a mayor concerns with this not being generic.
Meaning, it is easy to break with new panels etc. (that will be implemented soon).
I would be better if you could base the implementation purely on the bounds of the scrolleable parent of the dialog.
That way you don't need any special code for panels etc.
The Widget system needs to be as generic as possible.

[timon-schelling]

@ayush80942 are you still interested in working on this?

[ayush80942]

Yes, I am figuring out a way to find bounds of the scrollable parent without distributing between panels. Would be able to make changes 1-2 days.

[ayush80942]

@timon-schelling I have tried to generalise the code by removing hardcoded -

const detailsPanel = document.querySelector('[data-subdivision-name="details"]');

I have used the bounds of scrollable parents as suggested. Please have a look if it woks fine now.

[timon-schelling]

!build

[github-actions]

📦 Build Complete for a94fd84
https://4d76f673.graphite.pages.dev

[timon-schelling]

Wasn't able to find more edge cases.
One last thing please don't remove all the comments.
Thanks for contributing.
This a big frontend PR @Keavon has the last call.

[timon-schelling]

And run eslint on the file.

[ayush80942]

I have added the comments back as-is. I have fixed the eslint warnings as well. I just wanted to inform that the behaviour is still not fine in safari browser which i just tested. There is a safari specific code added in the production code at lines 82-91:

// Mitigate a Safari rendering bug which clips the floating menu extending beyond a scrollable container.
// The bug is possibly related to <https://bugs.webkit.org/show_bug.cgi?id=160953>, but in our case it happens when `overflow` of a parent is `auto` rather than `hidden`.
if (browserVersion().toLowerCase().includes("safari")) {
    const scrollable = self?.closest("[data-scrollable-x], [data-scrollable-y]");
    if (scrollable instanceof HTMLElement) {
        // The issue exists when the container is set to `overflow: auto` but fine when `overflow: hidden`. So this workaround temporarily sets
        // the scrollable container to `overflow: hidden`, thus removing the scrollbars and ability to scroll until the floating menu is closed.
        scrollable.style.overflow = isOpen ? "hidden" : "";
    }
}

[ayush80942]

@timon-schelling did you get a chance to review this ?

[Keavon]

I believe this is in my lap now not his to finish off. There will be a delay before I can get to it, I'm sorry to say, due to current priorities and backlog. Feel free to tackle other Rust-related issues in the meantime. Thanks for your patience.