Skip to content

fix: scale selection label arrow for narrow labels #165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aidenybai merged 1 commit into from
Feb 7, 2026
Merged

fix: scale selection label arrow for narrow labels #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions packages/react-grab/src/components/selection-label/arrow.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import type { Component } from "solid-js";
import type { ArrowProps } from "../../types.js";
import { getArrowSize } from "../../utils/get-arrow-size.js";

export const Arrow: Component<ArrowProps> = (props) => {
const arrowColor = () => props.color ?? "white";
const isBottom = () => props.position === "bottom";
const arrowSize = () => getArrowSize(props.labelWidth ?? 0);

return (
<div
Expand All @@ -16,10 +18,14 @@ export const Arrow: Component<ArrowProps> = (props) => {
transform: isBottom()
? "translateX(-50%) translateY(-100%)"
: "translateX(-50%) translateY(100%)",
"border-left": "8px solid transparent",
"border-right": "8px solid transparent",
"border-bottom": isBottom() ? `8px solid ${arrowColor()}` : undefined,
"border-top": isBottom() ? undefined : `8px solid ${arrowColor()}`,
"border-left": `${arrowSize()}px solid transparent`,
"border-right": `${arrowSize()}px solid transparent`,
"border-bottom": isBottom()
? `${arrowSize()}px solid ${arrowColor()}`
: undefined,
"border-top": isBottom()
? undefined
: `${arrowSize()}px solid ${arrowColor()}`,
filter: isBottom()
? "drop-shadow(-1px -1px 0 rgba(0,0,0,0.06)) drop-shadow(1px -1px 0 rgba(0,0,0,0.06))"
: "drop-shadow(-1px 1px 0 rgba(0,0,0,0.06)) drop-shadow(1px 1px 0 rgba(0,0,0,0.06))",
Expand Down
15 changes: 12 additions & 3 deletions packages/react-grab/src/components/selection-label/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ import type { Component } from "solid-js";
import type { ArrowPosition, SelectionLabelProps } from "../../types.js";
import {
VIEWPORT_MARGIN_PX,
ARROW_HEIGHT_PX,
ARROW_CENTER_PERCENT,
ARROW_LABEL_MARGIN_PX,
LABEL_GAP_PX,
PANEL_STYLES,
} from "../../constants.js";
import { getArrowSize } from "../../utils/get-arrow-size.js";
import { isKeyboardEventTriggeredByInput } from "../../utils/is-keyboard-event-triggered-by-input.js";
import { cn } from "../../utils/cn.js";
import { getTagDisplay } from "../../utils/get-tag-display.js";
Expand All @@ -40,6 +40,7 @@ const DEFAULT_OFFSCREEN_POSITION = {

export const SelectionLabel: Component<SelectionLabelProps> = (props) => {
let containerRef: HTMLDivElement | undefined;
let panelRef: HTMLDivElement | undefined;
let inputRef: HTMLTextAreaElement | undefined;
let isTagCurrentlyHovered = false;
let lastValidPosition: {
Expand All @@ -53,6 +54,7 @@ export const SelectionLabel: Component<SelectionLabelProps> = (props) => {

const [measuredWidth, setMeasuredWidth] = createSignal(0);
const [measuredHeight, setMeasuredHeight] = createSignal(0);
const [panelWidth, setPanelWidth] = createSignal(0);
const [arrowPosition, setArrowPosition] =
createSignal<ArrowPosition>("bottom");
const [viewportVersion, setViewportVersion] = createSignal(0);
Expand Down Expand Up @@ -89,6 +91,9 @@ export const SelectionLabel: Component<SelectionLabelProps> = (props) => {
setMeasuredWidth(rect.width);
setMeasuredHeight(rect.height);
}
if (panelRef) {
setPanelWidth(panelRef.getBoundingClientRect().width);
}
};

const handleTagHoverChange = (hovered: boolean) => {
Expand Down Expand Up @@ -215,11 +220,13 @@ export const SelectionLabel: Component<SelectionLabelProps> = (props) => {
const selectionBottom = bounds.y + bounds.height;
const selectionTop = bounds.y;

const actualArrowHeight = getArrowSize(panelWidth());
Copy link
Contributor

@pullfrog pullfrog bot Feb 7, 2026

Choose a reason for hiding this comment

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

This uses panelWidth() which is 0 during initial render. The positioning calculations will use the wrong arrow height (8px fallback) on first render, potentially causing the gap between arrow and panel that the PR description says it eliminates.


// HACK: Use cursorX as anchor point, CSS transform handles centering via translateX(-50%)
// This avoids the flicker when content changes because centering doesn't depend on JS measurement
const anchorX = cursorX;
let edgeOffsetX = 0;
let positionTop = selectionBottom + ARROW_HEIGHT_PX + LABEL_GAP_PX;
let positionTop = selectionBottom + actualArrowHeight + LABEL_GAP_PX;

// Calculate edge clamping offset (only applied when we have valid measurements)
if (labelWidth > 0) {
Expand All @@ -234,7 +241,7 @@ export const SelectionLabel: Component<SelectionLabelProps> = (props) => {
}
}

const totalHeightNeeded = labelHeight + ARROW_HEIGHT_PX + LABEL_GAP_PX;
const totalHeightNeeded = labelHeight + actualArrowHeight + LABEL_GAP_PX;
const fitsBelow =
positionTop + labelHeight <= viewportHeight - VIEWPORT_MARGIN_PX;

Expand Down Expand Up @@ -371,6 +378,7 @@ export const SelectionLabel: Component<SelectionLabelProps> = (props) => {
position={arrowPosition()}
leftPercent={computedPosition().arrowLeftPercent}
leftOffsetPx={computedPosition().arrowLeftOffset}
labelWidth={panelWidth()}
Copy link
Contributor

@pullfrog pullfrog bot Feb 7, 2026

Choose a reason for hiding this comment

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

Critical bug: The Arrow component renders before panelRef is assigned (line 405). When Arrow initially renders, panelWidth() returns 0, causing getArrowSize(0) to return the default ARROW_HEIGHT_PX (8px) instead of the scaled size.

The arrow will only get the correct size on subsequent re-renders after measureContainer runs. On first render with short labels, you'll still see the overflow issue that this PR aims to fix.

Solution: Move the Arrow after the panelRef div, or add a conditional render that waits for panelWidth() > 0.

/>

<Show when={isCompletedStatus() && !props.error}>
Expand All @@ -394,6 +402,7 @@ export const SelectionLabel: Component<SelectionLabelProps> = (props) => {
</Show>

<div
ref={panelRef}
class={cn(
"contain-layout flex items-center gap-[5px] rounded-[10px] antialiased w-fit h-fit p-0 [font-synthesis:none] [corner-shape:superellipse(1.25)]",
PANEL_STYLES,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-grab/src/constants.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export const FROZEN_GLOW_COLOR = `rgba(${GRAB_PURPLE_RGB}, 0.15)`;
export const FROZEN_GLOW_EDGE_PX = 50;

export const ARROW_HEIGHT_PX = 8;
export const ARROW_MIN_SIZE_PX = 4;
export const ARROW_MAX_LABEL_WIDTH_RATIO = 0.2;
Copy link
Contributor

@pullfrog pullfrog bot Feb 7, 2026

Choose a reason for hiding this comment

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

The 20% ratio seems arbitrary. For a 30px label, arrow becomes 6px. Was this tested visually? Consider documenting the rationale.

export const ARROW_CENTER_PERCENT = 50;
export const ARROW_LABEL_MARGIN_PX = 16;
export const LABEL_GAP_PX = 4;
Expand Down
1 change: 1 addition & 0 deletions packages/react-grab/src/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -544,6 +544,7 @@ export interface ArrowProps {
leftPercent: number;
leftOffsetPx: number;
color?: string;
labelWidth?: number;
}

export interface TagBadgeProps {
Expand Down
11 changes: 11 additions & 0 deletions packages/react-grab/src/utils/get-arrow-size.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import {
ARROW_HEIGHT_PX,
ARROW_MIN_SIZE_PX,
ARROW_MAX_LABEL_WIDTH_RATIO,
} from "../constants.js";

export const getArrowSize = (labelWidth: number): number => {
if (labelWidth <= 0) return ARROW_HEIGHT_PX;
const scaledSize = labelWidth * ARROW_MAX_LABEL_WIDTH_RATIO;
return Math.max(ARROW_MIN_SIZE_PX, Math.min(ARROW_HEIGHT_PX, scaledSize));
Comment on lines +9 to +10
Copy link
Contributor

@pullfrog pullfrog bot Feb 7, 2026

Choose a reason for hiding this comment

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

For very narrow labels (15px wide), this produces 3px which gets clamped to 4px. Is 4px arrow still too large for the narrowest cases? The ratio and minimum should be validated against actual short tag names like p, b, i mentioned in the test plan.

};
Loading