fix(app): calm the segmented control's selected segment
The selected segment was an inverted fill — theme foreground behind surface0 text — which in dark mode dropped a white slab into thin chrome like the file toolbar, and the fully rounded corners made it read as a lozenge rather than a tab. It now uses a surface3 fill with foreground text, and each segment takes the corner radius of the button at its size tier. Segments also drop one padding step below the equivalent button. The gap between segments already supplies the visual padding, so matching button padding made the track wider than it needed to be and ate the breathing room around it in a 32px toolbar row. Selection changes colour only. No weight or size change, so labels never reflow as you switch segments. Carries the buttonControlHeight extraction that was already sitting in control-geometry.ts, since the two files could not be split apart cleanly for this commit. Its callers stay uncommitted.
This commit is contained in:
+1
-1
@@ -64,7 +64,7 @@ The button is `<Button>` (`packages/app/src/components/ui/button.tsx`). It has f
|
||||
|
||||
Sizes: `xs` for ultra-tight inline triggers. `sm` for any button sitting in a row. `md` is the page default. `lg` is reserved for large standalone CTAs.
|
||||
|
||||
Sizes are a shared contract across control kinds, defined once in `control-geometry.ts`: `xs` = 28px tall with `fontSize.xs` labels, `sm` = 32px with `fontSize.sm`, `md`/`lg` = 44px with `fontSize.sm`. `<SegmentedControl>` (`packages/app/src/components/ui/segmented-control.tsx`) takes the same `xs`/`sm`/`md` sizes — a segmented control next to a `<Button>` of the same size always matches in height, label size, and horizontal padding. Thin chrome such as the file toolbar uses `xs`; settings rows use `sm`. Never shrink a control's font or padding locally to fit a context — if the context needs a smaller control, the size tier is missing or the wrong one is in use.
|
||||
Sizes are a shared contract across control kinds, defined once in `control-geometry.ts`: `xs` = 28px tall with `fontSize.xs` labels, `sm` = 32px with `fontSize.sm`, `md`/`lg` = 44px with `fontSize.sm`. `<SegmentedControl>` (`packages/app/src/components/ui/segmented-control.tsx`) takes the same `xs`/`sm`/`md` sizes — a segmented control next to a `<Button>` of the same size always matches in height, label size, and corner radius. Its segments run one padding step tighter than a button, because the gap between segments already reads as padding. The selected segment is a `surface3` fill with `foreground` text, not an inverted one — inverting it inside thin chrome puts a white slab in the toolbar. Thin chrome such as the file toolbar uses `xs`; settings rows use `sm`. Never shrink a control's font or padding locally to fit a context — if the context needs a smaller control, the size tier is missing or the wrong one is in use.
|
||||
|
||||
A `<Pressable>` wrapping a `<Text>` is a sixth variant. It is wrong. `<Button>` accepts `style`, `textStyle`, `leftIcon`, `disabled`, `size`, and `variant`.
|
||||
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
buttonControlHeight,
|
||||
createControlGeometry,
|
||||
getControlInteractionPhase,
|
||||
} from "@/components/ui/control-geometry";
|
||||
@@ -29,6 +30,7 @@ const theme = {
|
||||
},
|
||||
spacing: {
|
||||
0: 0,
|
||||
2: 8,
|
||||
3: 12,
|
||||
4: 16,
|
||||
6: 24,
|
||||
@@ -103,15 +105,15 @@ describe("control geometry", () => {
|
||||
expect(geometry.formTextInputMd.paddingVertical).toBe(11);
|
||||
});
|
||||
|
||||
it("keeps segmented controls ghost with fully rounded segments in a button-sized track", () => {
|
||||
it("keeps segmented controls ghost with button-radius segments in a button-sized track", () => {
|
||||
const geometry = createControlGeometry(theme);
|
||||
|
||||
expect(geometry.segmentedContainerXs.padding).toBe(0);
|
||||
expect(geometry.segmentedContainerSm.padding).toBe(0);
|
||||
expect(geometry.segmentedContainerMd.padding).toBe(0);
|
||||
expect(geometry.segmentedSegmentXs.borderRadius).toBe(9999);
|
||||
expect(geometry.segmentedSegmentSm.borderRadius).toBe(9999);
|
||||
expect(geometry.segmentedSegmentMd.borderRadius).toBe(9999);
|
||||
expect(geometry.segmentedSegmentXs.borderRadius).toBe(geometry.buttonXs.borderRadius);
|
||||
expect(geometry.segmentedSegmentSm.borderRadius).toBe(geometry.buttonSm.borderRadius);
|
||||
expect(geometry.segmentedSegmentMd.borderRadius).toBe(geometry.buttonMd.borderRadius);
|
||||
expect(geometry.segmentedContainerXs.minHeight).toBe(geometry.buttonXs.minHeight);
|
||||
expect(geometry.segmentedContainerSm.minHeight).toBe(geometry.buttonSm.minHeight);
|
||||
expect(geometry.segmentedContainerMd.minHeight).toBe(geometry.buttonMd.minHeight);
|
||||
@@ -127,6 +129,9 @@ describe("control geometry", () => {
|
||||
expect(geometry.buttonXs.minHeight).toBe(28);
|
||||
expect(geometry.buttonSm.minHeight).toBe(32);
|
||||
expect(geometry.buttonMd.minHeight).toBe(44);
|
||||
expect(geometry.buttonXs.minHeight).toBe(buttonControlHeight.xs);
|
||||
expect(geometry.buttonSm.minHeight).toBe(buttonControlHeight.sm);
|
||||
expect(geometry.buttonMd.minHeight).toBe(buttonControlHeight.md);
|
||||
|
||||
// Same size name means the same label size on every control kind.
|
||||
expect(geometry.segmentedLabelXs.fontSize).toBe(12);
|
||||
@@ -135,9 +140,16 @@ describe("control geometry", () => {
|
||||
expect(geometry.segmentedLabelSm.fontSize).toBe(geometry.buttonText.fontSize);
|
||||
expect(geometry.segmentedLabelMd.fontSize).toBe(geometry.buttonText.fontSize);
|
||||
|
||||
// Same size name means the same horizontal padding on every control kind.
|
||||
expect(geometry.segmentedSegmentXs.paddingHorizontal).toBe(geometry.buttonXs.paddingHorizontal);
|
||||
expect(geometry.segmentedSegmentSm.paddingHorizontal).toBe(geometry.buttonSm.paddingHorizontal);
|
||||
expect(geometry.segmentedSegmentMd.paddingHorizontal).toBe(geometry.buttonMd.paddingHorizontal);
|
||||
// Segments sit inside a track, so they run one padding step tighter than a
|
||||
// standalone button of the same size — the gap between segments reads as the padding.
|
||||
expect(geometry.segmentedSegmentXs.paddingHorizontal).toBeLessThan(
|
||||
geometry.buttonXs.paddingHorizontal,
|
||||
);
|
||||
expect(geometry.segmentedSegmentSm.paddingHorizontal).toBeLessThan(
|
||||
geometry.buttonSm.paddingHorizontal,
|
||||
);
|
||||
expect(geometry.segmentedSegmentMd.paddingHorizontal).toBeLessThan(
|
||||
geometry.buttonMd.paddingHorizontal,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -47,6 +47,13 @@ export const CONTROL_HEIGHTS = {
|
||||
field: FIELD_CONTROL_HEIGHT,
|
||||
};
|
||||
|
||||
export const buttonControlHeight: Record<ButtonControlSize, number> = {
|
||||
xs: CONTROL_HEIGHTS.tight,
|
||||
sm: CONTROL_HEIGHTS.compact,
|
||||
md: CONTROL_HEIGHTS.field,
|
||||
lg: CONTROL_HEIGHTS.field,
|
||||
};
|
||||
|
||||
export const buttonIconSize: Record<ButtonControlSize, number> = {
|
||||
xs: ICON_SIZE.xs,
|
||||
sm: ICON_SIZE.sm,
|
||||
@@ -133,22 +140,22 @@ export function createControlGeometry(theme: Theme) {
|
||||
|
||||
return {
|
||||
buttonXs: {
|
||||
minHeight: CONTROL_HEIGHTS.tight,
|
||||
minHeight: buttonControlHeight.xs,
|
||||
paddingHorizontal: theme.spacing[3],
|
||||
borderRadius: theme.borderRadius.md,
|
||||
},
|
||||
buttonSm: {
|
||||
minHeight: CONTROL_HEIGHTS.compact,
|
||||
minHeight: buttonControlHeight.sm,
|
||||
paddingHorizontal: theme.spacing[3],
|
||||
borderRadius: theme.borderRadius.md,
|
||||
},
|
||||
buttonMd: {
|
||||
minHeight: CONTROL_HEIGHTS.field,
|
||||
minHeight: buttonControlHeight.md,
|
||||
paddingHorizontal: theme.spacing[4],
|
||||
borderRadius: theme.borderRadius.lg,
|
||||
},
|
||||
buttonLg: {
|
||||
minHeight: CONTROL_HEIGHTS.field,
|
||||
minHeight: buttonControlHeight.lg,
|
||||
paddingHorizontal: theme.spacing[6],
|
||||
borderRadius: theme.borderRadius.xl,
|
||||
},
|
||||
@@ -211,18 +218,18 @@ export function createControlGeometry(theme: Theme) {
|
||||
},
|
||||
segmentedSegmentXs: {
|
||||
minHeight: CONTROL_HEIGHTS.tight - SEGMENTED_TIGHT_INSET * 2,
|
||||
paddingHorizontal: theme.spacing[3],
|
||||
borderRadius: theme.borderRadius.full,
|
||||
paddingHorizontal: theme.spacing[2],
|
||||
borderRadius: theme.borderRadius.md,
|
||||
},
|
||||
segmentedSegmentSm: {
|
||||
minHeight: CONTROL_HEIGHTS.compact - SEGMENTED_COMPACT_INSET * 2,
|
||||
paddingHorizontal: theme.spacing[3],
|
||||
borderRadius: theme.borderRadius.full,
|
||||
paddingHorizontal: theme.spacing[2],
|
||||
borderRadius: theme.borderRadius.md,
|
||||
},
|
||||
segmentedSegmentMd: {
|
||||
minHeight: CONTROL_HEIGHTS.field - SEGMENTED_FIELD_INSET * 2,
|
||||
paddingHorizontal: theme.spacing[4],
|
||||
borderRadius: theme.borderRadius.full,
|
||||
paddingHorizontal: theme.spacing[3],
|
||||
borderRadius: theme.borderRadius.lg,
|
||||
},
|
||||
segmentedLabelXs: {
|
||||
fontSize: theme.fontSize.xs,
|
||||
|
||||
@@ -41,7 +41,7 @@ function SegmentIcon({ icon, iconSize, iconColor }: SegmentIconProps) {
|
||||
|
||||
const ThemedSegmentIcon = withUnistyles(SegmentIcon);
|
||||
|
||||
const selectedIconMapping = (theme: Theme) => ({ iconColor: theme.colors.surface0 });
|
||||
const selectedIconMapping = (theme: Theme) => ({ iconColor: theme.colors.foreground });
|
||||
const mutedIconMapping = (theme: Theme) => ({ iconColor: theme.colors.foregroundMuted });
|
||||
|
||||
export function SegmentedControl<T extends string>({
|
||||
@@ -196,7 +196,7 @@ const styles = StyleSheet.create((theme) => {
|
||||
...geometry.segmentedSegmentMd,
|
||||
},
|
||||
segmentSelected: {
|
||||
backgroundColor: theme.colors.foreground,
|
||||
backgroundColor: theme.colors.surface3,
|
||||
},
|
||||
segmentHover: {
|
||||
backgroundColor: theme.colors.surface2,
|
||||
@@ -225,7 +225,7 @@ const styles = StyleSheet.create((theme) => {
|
||||
...geometry.segmentedLabelMd,
|
||||
},
|
||||
labelSelected: {
|
||||
color: theme.colors.surface0,
|
||||
color: theme.colors.foreground,
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user