mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-25 08:42:25 +00:00
999c4d5000
* Studio: stop a clicked tooltip from hanging over dialogs TooltipTrigger pinned a tooltip open on every click, including mouse clicks. That tap-to-pin is for touch, which has no hover. On a mouse it stranded tooltips: clicking a Model Hub row selected it and pinned its tooltip, and when a dialog then opened, Radix set pointer-events none outside the modal, the row stopped receiving pointerleave, and nothing could close the tooltip. It hung over the dialog until the page changed. - Only pin on a coarse pointer. With a mouse the handler returns early so Radix's own close-on-click runs, and the tooltip follows hover alone. - Release a pin on a press outside any tooltip, on Escape, and on window blur, so a pin can no longer outlive its interaction. Presses on a trigger are skipped so tapping the same one still toggles it shut. Fixed in the primitive, so it covers all five Hub call sites (CatalogRow, the three tooltips in models-table.tsx, and model-card.tsx) plus anywhere else with a clickable trigger. * Studio: force tooltips shut while a dialog is open Only pinned tooltips were released before. One opened by plain hover still hung over the dialog, because the pointer never moves when a dialog opens, so no hit-test happens and no pointerleave fires. Radix keeps the tooltip open. Radix sets body pointer-events to none for a modal layer, so observe that and force open to false while it holds, releasing back to hover when the dialog closes. One observer serves every tooltip. * Studio: read the pointer type from the click, not the primary device `(pointer: coarse)` describes the primary pointing device, so on a hybrid it mislabels every event from the other one: with a touchscreen primary a mouse click still pinned a tooltip and reproduced the dialog overlay, and with a mouse primary a tap no longer pinned. The click's own `pointerType` answers for the pointer actually used. Keyboard activation reports "", which correctly does not pin, and a webview with no PointerEvent still falls back to the media query. A controlled tooltip no longer receives the toggle at all. Its `open` comes from the consumer, so a pin could never render and only left state that blur and outside-press handlers were clearing for nothing. * Studio: keep modal tooltips interactive * Studio: take ownership of tooltip open state across a modal Three findings from the review, all confirmed. Ownership was read off the tooltip content. Radix ranks dismissable layers by mount order, so content opened after a modal reads pointer-events auto no matter where its trigger sits. The attachment tile is a concrete case: its TooltipTrigger is nested in a DialogTrigger, so one tap opens the lightbox and then mounts the tooltip above it, and the filename stayed over the image. The trigger answers this correctly and is mounted the whole time, so it is read instead. A trigger that drops the ref falls back to the whole modal, which leaves the tooltip shut rather than stranded. Forcing `open` to false only masked Radix's own state; it does not write to it. `useControllableState` leaves `uncontrolledProp` untouched while a value is supplied, so releasing back to `undefined` re-exposed the `true` from before the dialog. Hover a sidebar tooltip, open Settings with Cmd/Ctrl+comma, move into the dialog and close it, and the tooltip came back detached from its trigger. Hover is tracked here now and `open` is always supplied, so there is no second copy of the state to go stale, and Radix no longer warns about a controlled/uncontrolled switch. Controlled tooltips were excluded from blocking entirely. Their owners do not see the missing pointerleave either: `panel-resize-handle.tsx` sets `hovered` on pointerenter and clears it on pointerleave, so opening a dialog over a hovered handle left its z-[999999] tooltip on top of the dialog, which is the exact bug this PR exists to fix. Blocking now applies to them as well, and owners with an `onOpenChange` are told so their own state clears. * Studio: read modal ownership from the layer, not the trigger's own style Both findings are right, and the first is a regression the last commit introduced. Reading `pointer-events` off the trigger itself conflates an authored style with layer membership. `mcp-composer-button.tsx` has exactly that: the hint anchor inside each dropdown row is deliberately `pointer-events-none` so the row stays selectable, and under the modal dropdown it read as blocked, so the hint could never open. Radix writes `pointer-events` inline (none on the body, auto on the active layer, none on layers under it), so walking the trigger's ancestors for the nearest of those answers ownership without ever looking at the trigger's own class or style. The outside-press release matched triggers by `data-slot="tooltip-trigger"`, which an `asChild` child can drop: a component that does not spread its props never renders the attribute. A press on that trigger then read as outside, cleared the pin, and the click handler toggled it straight back on, so the tap never dismissed. It compares against the stored trigger element now. Content still matches by data-slot, which `TooltipContent` always renders itself and never delegates. Frontend 350/350, typecheck and build clean. Six tests over the layer walk, including the authored-none anchor and a layer beneath the modal. * Studio: keep a blocked controlled tooltip shut until its owner resets Right, and it is the residue the last commit left. `panel-resize-handle.tsx` passes `open` with no `onOpenChange`, so telling the owner the tooltip closed does nothing: its `hovered` is still true because the modal swallowed the pointerleave. Rendering `controlledOpen` again the moment the block lifted put the tooltip back with the pointer nowhere near the handle. A blocked controlled tooltip is now latched shut and only follows its owner again once that owner has said false at least once, which happens on the first pointer move after the dialog closes. Uncontrolled tooltips are not latched: their own hover state was already cleared, so a real hover shows them straight away. The whole decision is one pure function now, with tests for each case including the two that were wrong. --------- Co-authored-by: Unsloth <michaelhan@Michaels-MacBook-Pro.local>
51 lines
1.9 KiB
TypeScript
51 lines
1.9 KiB
TypeScript
// SPDX-License-Identifier: AGPL-3.0-only
|
|
// Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0
|
|
|
|
import assert from "node:assert/strict";
|
|
import test from "node:test";
|
|
|
|
import { isBlockedByActiveModal } from "../src/components/ui/tooltip-modal-layer.ts";
|
|
|
|
/** A trigger plus its ancestors, nearest first, by inline pointer-events. */
|
|
function trigger(own: string, ...ancestors: string[]): HTMLElement {
|
|
let parent: HTMLElement | null = null;
|
|
for (const pointerEvents of [...ancestors].reverse()) {
|
|
const node = { style: { pointerEvents }, parentElement: parent };
|
|
parent = node as unknown as HTMLElement;
|
|
}
|
|
return {
|
|
style: { pointerEvents: own },
|
|
parentElement: parent,
|
|
} as unknown as HTMLElement;
|
|
}
|
|
|
|
test("a trigger under the modal is blocked by the body", () => {
|
|
// sidebar button -> ... -> body(none)
|
|
assert.equal(isBlockedByActiveModal(trigger("", "none")), true);
|
|
});
|
|
|
|
test("a trigger inside the active layer is not blocked", () => {
|
|
// dialog content(auto) sits between the trigger and body(none)
|
|
assert.equal(isBlockedByActiveModal(trigger("", "auto", "none")), false);
|
|
});
|
|
|
|
test("a trigger on a layer beneath the modal is blocked", () => {
|
|
assert.equal(isBlockedByActiveModal(trigger("", "none", "none")), true);
|
|
});
|
|
|
|
test("no modal anywhere means nothing is blocked", () => {
|
|
assert.equal(isBlockedByActiveModal(trigger("", "", "")), false);
|
|
});
|
|
|
|
test("the trigger's own pointer-events is not modal ownership", () => {
|
|
// The MCP dropdown hint anchor is authored pointer-events-none so the row
|
|
// stays clickable. It is still inside the dropdown's layer.
|
|
assert.equal(isBlockedByActiveModal(trigger("none", "auto", "none")), false);
|
|
});
|
|
|
|
test("a detached element answers no rather than throwing", () => {
|
|
assert.equal(
|
|
isBlockedByActiveModal({ parentElement: null } as unknown as HTMLElement),
|
|
false,
|
|
);
|
|
});
|