mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: resolve outsideBox style for tabs (#24561)
> 🤖 This PR was modified by Coder Agents on behalf of Jake Howell.
Fixes the `outsideBox` variant styling for tabs and simplifies the kebab
overflow logic. The overflow calculation now accounts for `column-gap`
between tabs so the menu trigger appears at the correct breakpoint.
- Fix Tailwind hover selector syntax for `outsideBox` variant
(`[&_[data-slot=tabs-trigger]:hover]` instead of
`[&_[data-slot=tabs-trigger]]:hover`)
- Account for `column-gap` in `useKebabMenu` overflow calculation via a
new `getTabGap` helper
- Use content-box width (`getContentBoxWidth`) for the initial overflow
pass so it matches `ResizeObserver`'s `contentRect.width`
- Consolidate `calculateTabValues` into a single-pass loop, removing the
separate `findFirstTabIndex` function
- Drop `FC` wrapper in favor of inline prop destructuring across all tab
components
- Forward `ref` through `TabsList` so consumers can attach refs directly
- Add `useKebabMenu` unit tests covering both all-visible and overflow
scenarios
This commit is contained in:
@@ -3,7 +3,6 @@ import { Tabs as TabsPrimitive } from "radix-ui";
|
||||
import {
|
||||
type ComponentProps,
|
||||
createContext,
|
||||
type FC,
|
||||
type HTMLAttributes,
|
||||
useCallback,
|
||||
useContext,
|
||||
@@ -18,7 +17,7 @@ import { cn } from "#/utils/cn";
|
||||
|
||||
type TabsProps = ComponentProps<typeof TabsPrimitive.Root>;
|
||||
|
||||
export const Tabs: FC<TabsProps> = ({ ...props }) => {
|
||||
export const Tabs = ({ ...props }: TabsProps) => {
|
||||
return <TabsPrimitive.Root data-slot="tabs" {...props} />;
|
||||
};
|
||||
|
||||
@@ -39,7 +38,7 @@ const tabsListVariants = cva("flex flex-wrap items-center", {
|
||||
"[&_[data-slot=tabs-trigger]]:text-content-secondary [&_[data-slot=tabs-trigger][data-state=active]]:text-content-primary",
|
||||
"[&_[data-slot=tabs-trigger]]:border-0 [&_[data-slot=tabs-trigger]]:border-y [&_[data-slot=tabs-trigger]]:border-solid",
|
||||
"[&_[data-slot=tabs-trigger]]:border-transparent [&_[data-slot=tabs-trigger][data-state=active]]:border-b-white",
|
||||
"[&_[data-slot=tabs-trigger]]:hover:text-content-primary",
|
||||
"[&_[data-slot=tabs-trigger]:hover]:text-content-primary",
|
||||
"[&_[data-slot=tabs-trigger]]:px-1",
|
||||
),
|
||||
},
|
||||
@@ -53,14 +52,16 @@ type TabsListProps = ComponentProps<typeof TabsPrimitive.List> &
|
||||
overflowKebabMenu?: boolean;
|
||||
};
|
||||
|
||||
export const TabsList: FC<TabsListProps> = ({
|
||||
export const TabsList = ({
|
||||
className,
|
||||
variant,
|
||||
overflowKebabMenu = false,
|
||||
ref,
|
||||
...props
|
||||
}) => {
|
||||
}: TabsListProps) => {
|
||||
return (
|
||||
<TabsPrimitive.List
|
||||
ref={ref}
|
||||
data-slot="tabs-list"
|
||||
className={cn(
|
||||
tabsListVariants({ variant }),
|
||||
@@ -74,10 +75,10 @@ export const TabsList: FC<TabsListProps> = ({
|
||||
|
||||
type TabsTriggerProps = ComponentProps<typeof TabsPrimitive.Trigger>;
|
||||
|
||||
export const TabsTrigger: FC<TabsTriggerProps> = ({
|
||||
export const TabsTrigger = ({
|
||||
type: triggerType = "button",
|
||||
...props
|
||||
}) => {
|
||||
}: TabsTriggerProps) => {
|
||||
const type = props.asChild ? undefined : triggerType;
|
||||
|
||||
return (
|
||||
@@ -85,11 +86,12 @@ export const TabsTrigger: FC<TabsTriggerProps> = ({
|
||||
data-slot="tabs-trigger"
|
||||
type={type}
|
||||
className={cn(
|
||||
"border-none py-3 bg-transparent",
|
||||
"border-none py-2.5 bg-transparent",
|
||||
"text-inherit font-normal text-sm",
|
||||
"inline-flex gap-2 items-center",
|
||||
"cursor-pointer",
|
||||
"transition-colors duration-150 ease-linear",
|
||||
"-mb-px",
|
||||
)}
|
||||
{...props}
|
||||
/>
|
||||
@@ -98,7 +100,7 @@ export const TabsTrigger: FC<TabsTriggerProps> = ({
|
||||
|
||||
type TabsContentProps = ComponentProps<typeof TabsPrimitive.Content>;
|
||||
|
||||
export const TabsContent: FC<TabsContentProps> = ({ ...props }) => {
|
||||
export const TabsContent = ({ ...props }: TabsContentProps) => {
|
||||
return <TabsPrimitive.Content data-slot="tabs-content" {...props} />;
|
||||
};
|
||||
|
||||
@@ -117,11 +119,11 @@ const LinkTabsContext = createContext<LinkTabsContextValue | undefined>(
|
||||
|
||||
type LinkTabsProps = HTMLAttributes<HTMLDivElement> & LinkTabsContextValue;
|
||||
|
||||
export const LinkTabs: FC<LinkTabsProps> = ({
|
||||
export const LinkTabs = ({
|
||||
className,
|
||||
active,
|
||||
...htmlProps
|
||||
}) => {
|
||||
}: LinkTabsProps) => {
|
||||
return (
|
||||
<LinkTabsContext.Provider value={{ active }}>
|
||||
<div
|
||||
@@ -140,10 +142,7 @@ export const LinkTabs: FC<LinkTabsProps> = ({
|
||||
|
||||
type LinkTabsListProps = HTMLAttributes<HTMLDivElement>;
|
||||
|
||||
export const LinkTabsList: FC<LinkTabsListProps> = ({
|
||||
className,
|
||||
...props
|
||||
}) => {
|
||||
export const LinkTabsList = ({ className, ...props }: LinkTabsListProps) => {
|
||||
const tabsContext = useContext(LinkTabsContext);
|
||||
const listRef = useRef<HTMLDivElement>(null);
|
||||
const indicatorRef = useRef<HTMLDivElement>(null);
|
||||
@@ -217,11 +216,7 @@ type TabLinkProps = LinkProps & {
|
||||
value: string;
|
||||
};
|
||||
|
||||
export const TabLink: FC<TabLinkProps> = ({
|
||||
value,
|
||||
className,
|
||||
...linkProps
|
||||
}) => {
|
||||
export const TabLink = ({ value, className, ...linkProps }: TabLinkProps) => {
|
||||
const tabsContext = useContext(LinkTabsContext);
|
||||
if (!tabsContext) {
|
||||
throw new Error("TabLink must be used inside LinkTabs");
|
||||
|
||||
@@ -0,0 +1,134 @@
|
||||
import { act, render, screen } from "@testing-library/react";
|
||||
import { useKebabMenu } from "./useKebabMenu";
|
||||
|
||||
type FakeResizeObserverInstance = {
|
||||
simulateResize: (width: number) => void;
|
||||
};
|
||||
|
||||
let resizeObserverInstances: FakeResizeObserverInstance[] = [];
|
||||
|
||||
class MockResizeObserver {
|
||||
private readonly callback: ResizeObserverCallback;
|
||||
|
||||
constructor(callback: ResizeObserverCallback) {
|
||||
this.callback = callback;
|
||||
const self = this;
|
||||
resizeObserverInstances.push({
|
||||
simulateResize(width: number) {
|
||||
self.callback(
|
||||
[{ contentRect: { width, height: 0 } } as ResizeObserverEntry],
|
||||
self as unknown as ResizeObserver,
|
||||
);
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
observe(_target: Element) {}
|
||||
unobserve(_target: Element) {}
|
||||
disconnect() {}
|
||||
}
|
||||
|
||||
const getLastResizeObserver = (): FakeResizeObserverInstance => {
|
||||
const instance = resizeObserverInstances[resizeObserverInstances.length - 1];
|
||||
if (!instance) {
|
||||
throw new Error("No ResizeObserver was constructed");
|
||||
}
|
||||
return instance;
|
||||
};
|
||||
|
||||
const setElementOffsetWidth = (element: HTMLElement, width: number): void => {
|
||||
Object.defineProperty(element, "offsetWidth", {
|
||||
configurable: true,
|
||||
get: () => width,
|
||||
});
|
||||
};
|
||||
|
||||
const tabs = [
|
||||
{ value: "all", label: "All Logs" },
|
||||
{ value: "build", label: "Build Logs" },
|
||||
{ value: "startup", label: "Startup Script" },
|
||||
] as const;
|
||||
|
||||
const TestHarness = ({ tabGap = 0 }: { tabGap?: number }) => {
|
||||
const { containerRef, visibleTabs, overflowTabs, getTabMeasureProps } =
|
||||
useKebabMenu({
|
||||
tabs,
|
||||
enabled: true,
|
||||
isActive: true,
|
||||
overflowTriggerWidth: 44,
|
||||
});
|
||||
|
||||
return (
|
||||
<div>
|
||||
<div
|
||||
ref={containerRef}
|
||||
style={{ display: "flex", columnGap: `${tabGap}px` }}
|
||||
>
|
||||
{tabs.map((tab) => (
|
||||
<button
|
||||
key={tab.value}
|
||||
type="button"
|
||||
{...getTabMeasureProps(tab.value)}
|
||||
>
|
||||
{tab.label}
|
||||
</button>
|
||||
))}
|
||||
</div>
|
||||
<div data-testid="visible-values">
|
||||
{visibleTabs.map((tab) => tab.value).join(",")}
|
||||
</div>
|
||||
<div data-testid="overflow-values">
|
||||
{overflowTabs.map((tab) => tab.value).join(",")}
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
};
|
||||
|
||||
describe("useKebabMenu", () => {
|
||||
beforeEach(() => {
|
||||
resizeObserverInstances = [];
|
||||
vi.stubGlobal("ResizeObserver", MockResizeObserver);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Keep tests isolated when other suites spy on globals.
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllGlobals();
|
||||
});
|
||||
|
||||
it("shows all tabs when the available width is enough", async () => {
|
||||
render(<TestHarness />);
|
||||
|
||||
const [all, build, startup] = screen.getAllByRole("button");
|
||||
setElementOffsetWidth(all, 60);
|
||||
setElementOffsetWidth(build, 70);
|
||||
setElementOffsetWidth(startup, 70);
|
||||
|
||||
await act(() => {
|
||||
getLastResizeObserver().simulateResize(220);
|
||||
});
|
||||
|
||||
expect(screen.getByTestId("visible-values")).toHaveTextContent(
|
||||
"all,build,startup",
|
||||
);
|
||||
expect(screen.getByTestId("overflow-values")).toBeEmptyDOMElement();
|
||||
});
|
||||
|
||||
it("accounts for outsideBox tab gap when reserving kebab space", async () => {
|
||||
render(<TestHarness tabGap={24} />);
|
||||
|
||||
const [all, build, startup] = screen.getAllByRole("button");
|
||||
setElementOffsetWidth(all, 60);
|
||||
setElementOffsetWidth(build, 70);
|
||||
setElementOffsetWidth(startup, 70);
|
||||
|
||||
await act(() => {
|
||||
getLastResizeObserver().simulateResize(220);
|
||||
});
|
||||
|
||||
expect(screen.getByTestId("visible-values")).toHaveTextContent("all");
|
||||
expect(screen.getByTestId("overflow-values")).toHaveTextContent(
|
||||
"build,startup",
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -1,7 +1,6 @@
|
||||
import {
|
||||
type RefObject,
|
||||
useCallback,
|
||||
useEffect,
|
||||
useLayoutEffect,
|
||||
useRef,
|
||||
useState,
|
||||
@@ -41,10 +40,6 @@ export const useKebabMenu = <T extends TabValue>({
|
||||
overflowTriggerWidth = 44,
|
||||
}: UseKebabMenuOptions<T>): UseKebabMenuResult<T> => {
|
||||
const containerRef = useRef<HTMLDivElement>(null);
|
||||
const tabsRef = useRef<readonly T[]>(tabs);
|
||||
tabsRef.current = tabs;
|
||||
const previousTabsRef = useRef<readonly T[]>(tabs);
|
||||
const availableWidthRef = useRef<number | null>(null);
|
||||
// Width cache prevents oscillation when overflow tabs are not mounted.
|
||||
const tabWidthByValueRef = useRef<Record<string, number>>({});
|
||||
const [overflowTabValues, setTabValues] = useState<string[]>([]);
|
||||
@@ -66,20 +61,20 @@ export const useKebabMenu = <T extends TabValue>({
|
||||
if (!container) {
|
||||
return;
|
||||
}
|
||||
const currentTabs = tabsRef.current;
|
||||
|
||||
const tabWidthByValue = measureTabWidths({
|
||||
tabs: currentTabs,
|
||||
tabs,
|
||||
container,
|
||||
previousTabWidthByValue: tabWidthByValueRef.current,
|
||||
});
|
||||
tabWidthByValueRef.current = tabWidthByValue;
|
||||
const tabGap = getTabGap(container);
|
||||
|
||||
const nextOverflowValues = calculateTabValues({
|
||||
tabs: currentTabs,
|
||||
tabs,
|
||||
availableWidth,
|
||||
tabWidthByValue,
|
||||
overflowTriggerWidth,
|
||||
tabGap,
|
||||
});
|
||||
|
||||
setTabValues((currentValues) => {
|
||||
@@ -90,35 +85,34 @@ export const useKebabMenu = <T extends TabValue>({
|
||||
return nextOverflowValues;
|
||||
});
|
||||
},
|
||||
[enabled, isActive, overflowTriggerWidth],
|
||||
[enabled, isActive, overflowTriggerWidth, tabs],
|
||||
);
|
||||
|
||||
useEffect(() => {
|
||||
if (previousTabsRef.current === tabs) {
|
||||
// No change in tabs, no need to recalculate.
|
||||
return;
|
||||
}
|
||||
previousTabsRef.current = tabs;
|
||||
if (availableWidthRef.current === null) {
|
||||
// First mount, no width available yet.
|
||||
return;
|
||||
}
|
||||
recalculateOverflow(availableWidthRef.current);
|
||||
}, [recalculateOverflow, tabs]);
|
||||
|
||||
useLayoutEffect(() => {
|
||||
const container = containerRef.current;
|
||||
if (!container || !enabled || !isActive) {
|
||||
if (!enabled || !isActive) {
|
||||
// Keep this update idempotent to avoid render loops.
|
||||
setTabValues((currentValues) => {
|
||||
if (currentValues.length === 0) {
|
||||
return currentValues;
|
||||
}
|
||||
return [];
|
||||
});
|
||||
return;
|
||||
}
|
||||
if (!container) {
|
||||
return;
|
||||
}
|
||||
|
||||
recalculateOverflow(getContentBoxWidth(container));
|
||||
|
||||
// Recompute whenever ResizeObserver reports a container width change.
|
||||
const observer = new ResizeObserver(([entry]) => {
|
||||
if (!entry) {
|
||||
return;
|
||||
}
|
||||
availableWidthRef.current = entry.contentRect.width;
|
||||
recalculateOverflow(entry.contentRect.width);
|
||||
const nextAvailableWidth = Math.max(0, entry.contentRect.width);
|
||||
recalculateOverflow(nextAvailableWidth);
|
||||
});
|
||||
observer.observe(container);
|
||||
return () => observer.disconnect();
|
||||
@@ -157,47 +151,40 @@ const calculateTabValues = <T extends TabValue>({
|
||||
availableWidth,
|
||||
tabWidthByValue,
|
||||
overflowTriggerWidth,
|
||||
tabGap,
|
||||
}: {
|
||||
tabs: readonly T[];
|
||||
availableWidth: number;
|
||||
tabWidthByValue: Readonly<Record<string, number>>;
|
||||
overflowTriggerWidth: number;
|
||||
tabGap: number;
|
||||
}): string[] => {
|
||||
const tabWidthByValueMap = new Map<string, number>();
|
||||
for (const tab of tabs) {
|
||||
tabWidthByValueMap.set(tab.value, tabWidthByValue[tab.value] ?? 0);
|
||||
}
|
||||
|
||||
const firstOptionalTabIndex = Math.min(
|
||||
ALWAYS_VISIBLE_TABS_COUNT,
|
||||
tabs.length,
|
||||
);
|
||||
if (firstOptionalTabIndex >= tabs.length) {
|
||||
if (tabs.length <= ALWAYS_VISIBLE_TABS_COUNT) {
|
||||
return [];
|
||||
}
|
||||
|
||||
const alwaysVisibleTabs = tabs.slice(0, firstOptionalTabIndex);
|
||||
const optionalTabs = tabs.slice(firstOptionalTabIndex);
|
||||
const alwaysVisibleWidth = alwaysVisibleTabs.reduce((total, tab) => {
|
||||
return total + (tabWidthByValueMap.get(tab.value) ?? 0);
|
||||
}, 0);
|
||||
const firstTabIndex = findFirstTabIndex({
|
||||
optionalTabs,
|
||||
optionalTabWidths: optionalTabs.map((tab) => {
|
||||
return tabWidthByValueMap.get(tab.value) ?? 0;
|
||||
}),
|
||||
startingUsedWidth: alwaysVisibleWidth,
|
||||
availableWidth,
|
||||
overflowTriggerWidth,
|
||||
});
|
||||
let usedWidth = 0;
|
||||
let visibleCount = 0;
|
||||
|
||||
if (firstTabIndex === -1) {
|
||||
return [];
|
||||
for (const [index, tab] of tabs.entries()) {
|
||||
const tabWidth = tabWidthByValue[tab.value] ?? 0;
|
||||
const gapBeforeTab = visibleCount > 0 ? tabGap : 0;
|
||||
const usedWidthWithTab = usedWidth + gapBeforeTab + tabWidth;
|
||||
const hasMoreTabs = index < tabs.length - 1;
|
||||
// Reserve kebab trigger width whenever additional tabs remain.
|
||||
const widthNeeded =
|
||||
usedWidthWithTab + (hasMoreTabs ? tabGap + overflowTriggerWidth : 0);
|
||||
|
||||
if (index < ALWAYS_VISIBLE_TABS_COUNT || widthNeeded <= availableWidth) {
|
||||
usedWidth = usedWidthWithTab;
|
||||
visibleCount += 1;
|
||||
continue;
|
||||
}
|
||||
|
||||
return tabs.slice(index).map((overflowTab) => overflowTab.value);
|
||||
}
|
||||
|
||||
return optionalTabs
|
||||
.slice(firstTabIndex)
|
||||
.map((overflowTab) => overflowTab.value);
|
||||
return [];
|
||||
};
|
||||
|
||||
const measureTabWidths = <T extends TabValue>({
|
||||
@@ -221,47 +208,17 @@ const measureTabWidths = <T extends TabValue>({
|
||||
return nextTabWidthByValue;
|
||||
};
|
||||
|
||||
const findFirstTabIndex = ({
|
||||
optionalTabs,
|
||||
optionalTabWidths,
|
||||
startingUsedWidth,
|
||||
availableWidth,
|
||||
overflowTriggerWidth,
|
||||
}: {
|
||||
optionalTabs: readonly TabValue[];
|
||||
optionalTabWidths: readonly number[];
|
||||
startingUsedWidth: number;
|
||||
availableWidth: number;
|
||||
overflowTriggerWidth: number;
|
||||
}): number => {
|
||||
const result = optionalTabs.reduce(
|
||||
(acc, _tab, index) => {
|
||||
if (acc.firstTabIndex !== -1) {
|
||||
return acc;
|
||||
}
|
||||
const getContentBoxWidth = (container: HTMLElement): number => {
|
||||
const styles = window.getComputedStyle(container);
|
||||
const paddingLeft = Number.parseFloat(styles.paddingLeft) || 0;
|
||||
const paddingRight = Number.parseFloat(styles.paddingRight) || 0;
|
||||
return container.clientWidth - paddingLeft - paddingRight;
|
||||
};
|
||||
|
||||
const tabWidth = optionalTabWidths[index] ?? 0;
|
||||
const hasMoreTabs = index < optionalTabs.length - 1;
|
||||
// Reserve kebab trigger width whenever additional tabs remain.
|
||||
const widthNeeded =
|
||||
acc.usedWidth + tabWidth + (hasMoreTabs ? overflowTriggerWidth : 0);
|
||||
|
||||
if (widthNeeded <= availableWidth) {
|
||||
return {
|
||||
usedWidth: acc.usedWidth + tabWidth,
|
||||
firstTabIndex: -1,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
usedWidth: acc.usedWidth,
|
||||
firstTabIndex: index,
|
||||
};
|
||||
},
|
||||
{ usedWidth: startingUsedWidth, firstTabIndex: -1 },
|
||||
);
|
||||
|
||||
return result.firstTabIndex;
|
||||
const getTabGap = (container: HTMLElement): number => {
|
||||
const styles = window.getComputedStyle(container);
|
||||
const gap = Number.parseFloat(styles.columnGap);
|
||||
return Number.isFinite(gap) ? gap : 0;
|
||||
};
|
||||
|
||||
const areStringArraysEqual = (
|
||||
|
||||
@@ -2,6 +2,7 @@ import Collapse from "@mui/material/Collapse";
|
||||
import {
|
||||
CopyIcon,
|
||||
EllipsisIcon,
|
||||
PackageIcon,
|
||||
PlayIcon,
|
||||
SquareCheckBigIcon,
|
||||
TriangleAlertIcon,
|
||||
@@ -280,6 +281,7 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
{
|
||||
title: "All Logs",
|
||||
value: "all",
|
||||
startIcon: <PackageIcon className="size-icon-xs shrink-0" />,
|
||||
},
|
||||
...(startupScriptLogTab ? [startupScriptLogTab] : []),
|
||||
...sortedSourceLogTabs,
|
||||
@@ -535,11 +537,13 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
onValueChange={setSelectedLogTab}
|
||||
>
|
||||
<div className="flex items-stretch">
|
||||
<div
|
||||
ref={logTabsListContainerRef}
|
||||
className="min-w-0 flex-1 overflow-hidden"
|
||||
>
|
||||
<TabsList variant="insideBox" overflowKebabMenu>
|
||||
<div className="min-w-0 flex-1 overflow-hidden">
|
||||
<TabsList
|
||||
variant="outsideBox"
|
||||
overflowKebabMenu
|
||||
ref={logTabsListContainerRef}
|
||||
className="px-4"
|
||||
>
|
||||
{visibleLogTabs.map((tab) => (
|
||||
<TabsTrigger
|
||||
key={tab.value}
|
||||
@@ -565,7 +569,12 @@ export const AgentRow: FC<AgentRowProps> = ({
|
||||
: "inactive"
|
||||
}
|
||||
aria-label="More log tabs"
|
||||
className="border-none py-4 bg-transparent text-inherit inline-flex items-center justify-center cursor-pointer transition-colors duration-150 ease-linear"
|
||||
className={cn(
|
||||
"cursor-pointer -mb-px",
|
||||
"inline-flex items-center justify-center",
|
||||
"border-none py-3 bg-transparent text-inherit",
|
||||
"transition-colors duration-150 ease-linear",
|
||||
)}
|
||||
>
|
||||
<EllipsisIcon className="size-icon-sm" />
|
||||
<span className="sr-only">More log tabs</span>
|
||||
|
||||
Reference in New Issue
Block a user