diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index 37c62921c..622c931cf 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -105,7 +105,6 @@ import { useLimitedSidebarGroup } from "@/components/sidebar/use-limited-sidebar import { SidebarWorkspaceRowFrame, SidebarWorkspaceRowContent, - sidebarWorkspaceRowStyles, SidebarWorkspaceShortcutBadge, resolveTrailingActionVisibility, SidebarWorkspaceTrailingActionBase, @@ -113,7 +112,7 @@ import { SidebarWorkspaceTrailingActionSlot, } from "@/components/sidebar/sidebar-workspace-row-content"; import { useOpenKebabMenuVisibility } from "@/components/sidebar/use-open-kebab-menu-visibility"; -import { selectWorkspaceScriptSummary } from "@/components/sidebar/workspace-meta-row"; +import { selectWorkspaceServiceSummary } from "@/components/sidebar/workspace-meta-row"; import { SidebarWorkspaceTrailingContent, useSidebarWorkspaceTrailing, @@ -304,9 +303,6 @@ interface WorkspaceRowInnerProps { isPinned?: boolean; onTogglePin?: () => void; reserveIdleStatusIndicatorSpace?: boolean; - /** Row sits under a group header (a project row). Pinned rows are not grouped. - * Drives the indent that lands the row on the header label's rail. */ - inGroup?: boolean; } export function PrBadge({ hint, style }: { hint: PrHint; style?: StyleProp }) { @@ -382,16 +378,13 @@ function getProjectWorkspaceRowStyle({ isDragging, selected, isHovered, - inGroup, }: { isDragging: boolean; selected: boolean; isHovered: boolean; - inGroup: boolean; }) { return [ styles.workspaceRow, - inGroup && sidebarWorkspaceRowStyles.rowIndented, isDragging && styles.workspaceRowDragging, selected && styles.sidebarRowSelected, isHovered && styles.workspaceRowHovered, @@ -815,7 +808,8 @@ function NewWorkspaceGhostRow({ const rowStyle = useCallback( ({ hovered = false, pressed }: PressableStateCallbackType & { hovered?: boolean }) => [ styles.newWorkspaceGhostRow, - (Boolean(hovered) || pressed) && styles.newWorkspaceGhostRowHovered, + hovered && !pressed && styles.newWorkspaceGhostRowHovered, + pressed && styles.newWorkspaceGhostRowPressed, ], [], ); @@ -1068,7 +1062,6 @@ function WorkspaceRowInner({ isPinned, onTogglePin, reserveIdleStatusIndicatorSpace = true, - inGroup = false, }: WorkspaceRowInnerProps) { const _isCompact = useIsCompactFormFactor(); const isTouchPlatform = platformIsNative; @@ -1097,12 +1090,11 @@ function WorkspaceRowInner({ {({ isHovered, contextMenuOpen, onContextMenuOpenChange, hoverHandlers }) => { const isDesktop = !isTouchPlatform; - const scriptSummary = isDesktop ? selectWorkspaceScriptSummary(workspace.scripts) : null; + const serviceSummary = isDesktop ? selectWorkspaceServiceSummary(workspace.scripts) : null; const workspaceRowStyle = getProjectWorkspaceRowStyle({ isDragging, selected, isHovered, - inGroup, }); return ( ); @@ -1658,7 +1634,6 @@ function ProjectBlock({ workspace={item} workspaceEntry={workspaceEntriesByKey.get(item.workspaceKey) ?? null} hostBadge={hostBadgeByServerId.get(item.serverId) ?? null} - inGroup shortcutNumber={shortcutIndexByWorkspaceKey.get(item.workspaceKey) ?? null} showShortcutBadge={showShortcutBadges} canCopyBranchName={project.projectKind === "git"} @@ -1813,7 +1788,11 @@ function ProjectBlock({ } return ( - + ({ pinnedSection: { marginBottom: theme.spacing[1], }, - projectBlock: { - marginBottom: theme.spacing[1], + // Three times the gap a row keeps from its neighbour, so the break between two groups reads as + // a break rather than as one more row of pitch. Kept equal to `statusGroupBlockExpanded` — the + // two groupings are the same list under a different heading and must not breathe differently. + // + // Padding on the block rather than margin, and only while it has children: the gap belongs to + // the rows underneath the header, so a collapsed project gives it back and a column of collapsed + // headers closes up to the pitch of a list instead of staying spaced for content that is gone. + projectBlockExpanded: { + paddingBottom: theme.spacing[3], }, workspaceListContainer: {}, + // Kept in step with `workspaceRow` above. It stands in a project's list where a workspace row + // would be, so it takes that row's geometry and both of its fills. newWorkspaceGhostRow: { - minHeight: 32, - marginLeft: theme.spacing[6], - marginRight: theme.spacing[1], - paddingVertical: theme.spacing[1], - paddingHorizontal: theme.spacing[2], - borderRadius: theme.borderRadius.md, + minHeight: 36, + marginBottom: theme.spacing[0.5], + paddingVertical: theme.spacing[2], + paddingLeft: theme.spacing[2], + paddingRight: theme.spacing[3], + borderRadius: theme.borderRadius.lg, flexDirection: "row", alignItems: "center", gap: theme.spacing[2], @@ -2442,9 +2430,14 @@ const styles = StyleSheet.create((theme) => ({ newWorkspaceGhostRowHovered: { backgroundColor: theme.colors.surfaceSidebarHover, }, + newWorkspaceGhostRowPressed: { + backgroundColor: theme.colors.surface2, + }, + // The width of a workspace row's status slot, so the label lands on the same rail as the + // titles above it. newWorkspaceGhostIconSlot: { - width: theme.iconSize.sm, - height: theme.iconSize.sm, + width: theme.iconSize.md, + height: theme.iconSize.md, alignItems: "center", justifyContent: "center", flexShrink: 0, @@ -2608,7 +2601,7 @@ const styles = StyleSheet.create((theme) => ({ }, workspaceRow: { minHeight: 36, - marginBottom: theme.spacing[1], + marginBottom: theme.spacing[0.5], paddingVertical: theme.spacing[2], paddingLeft: theme.spacing[2], paddingRight: theme.spacing[3], diff --git a/packages/app/src/components/sidebar/display-preferences/checks-display.ts b/packages/app/src/components/sidebar/display-preferences/checks-display.ts new file mode 100644 index 000000000..ae34b1e1c --- /dev/null +++ b/packages/app/src/components/sidebar/display-preferences/checks-display.ts @@ -0,0 +1,27 @@ +/** + * How much of a workspace's CI a sidebar row spends space on. + * + * Not one of the boolean row items, because the icon on its own is a real third answer rather + * than half of "off": the state stays on the row for a glance, and a passing workspace stops + * paying a word for it on every line. A boolean cannot say that, and widening the row-items + * record to hold it would make three settings carry a shape only one of them uses. + * + * Pure on purpose, like `row-items.ts`: `hooks/use-settings/storage.ts` validates the persisted + * value through `parseSidebarChecksDisplay` rather than growing its own copy of the value list. + */ + +export const SIDEBAR_CHECKS_DISPLAYS = ["iconAndText", "icon", "none"] as const; + +export type SidebarChecksDisplay = (typeof SIDEBAR_CHECKS_DISPLAYS)[number]; + +export const DEFAULT_SIDEBAR_CHECKS_DISPLAY: SidebarChecksDisplay = "iconAndText"; + +/** Null for anything that isn't one of the three, so callers can tell "absent" from "chosen". */ +export function parseSidebarChecksDisplay(value: unknown): SidebarChecksDisplay | null { + if (typeof value !== "string") { + return null; + } + return (SIDEBAR_CHECKS_DISPLAYS as readonly string[]).includes(value) + ? (value as SidebarChecksDisplay) + : null; +} diff --git a/packages/app/src/components/sidebar/display-preferences/menu.tsx b/packages/app/src/components/sidebar/display-preferences/menu.tsx index ace3b97c5..b0ba9ba52 100644 --- a/packages/app/src/components/sidebar/display-preferences/menu.tsx +++ b/packages/app/src/components/sidebar/display-preferences/menu.tsx @@ -3,16 +3,18 @@ import { useTranslation } from "react-i18next"; import { View, type PressableStateCallbackType } from "react-native"; import { StyleSheet, withUnistyles } from "react-native-unistyles"; import { + Captions, CircleCheck, CircleDashed, Clock, Diff, + EyeOff, Folder, GitBranch, GitPullRequest, + Globe, Server, Settings2, - SquareTerminal, Type, } from "lucide-react-native"; import { @@ -30,12 +32,15 @@ import { useHosts } from "@/runtime/host-runtime"; import type { Theme } from "@/styles/theme"; import type { SidebarGroupMode } from "@/stores/sidebar-view-store"; import type { WorkspaceTitleSource } from "@/hooks/use-settings"; +import { SIDEBAR_CHECKS_DISPLAYS, type SidebarChecksDisplay } from "./checks-display"; import { useSidebarDisplayPreferences, type SidebarTrailingChoice } from "./model"; import { SIDEBAR_ROW_ITEMS, type SidebarRowItem } from "./row-items"; const mutedIconMapping = (theme: Theme) => ({ color: theme.colors.foregroundMuted }); const ThemedSettings2 = withUnistyles(Settings2); +/** CI's mark: the subject of the checks row, and the shape the icon-only option leaves behind. */ +const ThemedCircleCheck = withUnistyles(CircleCheck); /** Fits the item's 16pt leading slot with a hair of room, matching the trailing check. */ const OPTION_ICON_SIZE = 14; @@ -63,8 +68,15 @@ const TITLE_SOURCE_ICONS: Record = { const ROW_ITEM_ICONS: Record = { host: withUnistyles(Server), changeRequest: withUnistyles(GitPullRequest), - checks: withUnistyles(CircleCheck), - scripts: withUnistyles(SquareTerminal), + services: withUnistyles(Globe), +}; + +// These mark how much of the row an option spends, not what CI is, so they are the shapes each +// answer produces: a glyph with words beside it, the glyph on its own, nothing. +const CHECKS_DISPLAY_ICONS: Record = { + iconAndText: withUnistyles(Captions), + icon: ThemedCircleCheck, + none: withUnistyles(EyeOff), }; const TRAILING_ICONS: Record = { @@ -89,8 +101,13 @@ const TITLE_SOURCE_LABEL_KEYS: Record = { const ROW_ITEM_LABEL_KEYS: Record = { host: "sidebar.display.show.host", changeRequest: "sidebar.display.show.changeRequest", - checks: "sidebar.display.show.checks", - scripts: "sidebar.display.show.scripts", + services: "sidebar.display.show.services", +}; + +const CHECKS_DISPLAY_LABEL_KEYS: Record = { + iconAndText: "sidebar.display.checks.iconAndText", + icon: "sidebar.display.checks.icon", + none: "sidebar.display.checks.none", }; const TRAILING_LABEL_KEYS: Record = { @@ -155,6 +172,20 @@ export function SidebarDisplayPreferencesMenu(): ReactElement { title: t("sidebar.display.show.label"), content: , }, + { + id: "checks", + title: t("sidebar.display.show.checks"), + content: ( + + ), + }, ]; if (showHostFilter) { @@ -292,6 +323,10 @@ function OptionList({ * Two groups, split by the separator. Above it, what a row may say about a workspace — each one * independent. Below it, the one thing the slot to the right of the title holds, so picking the * one already showing empties the slot and gives the width back to the title. + * + * CI is the one item above the separator with three answers rather than two, so it opens a page + * instead of ticking, and it goes last: a row that navigates does not belong in the middle of a + * column you are running down with your eyes ticking things on and off. */ function ShowPage({ preferences }: { preferences: Preferences }): ReactElement { const { t } = useTranslation(); @@ -309,6 +344,7 @@ function ShowPage({ preferences }: { preferences: Preferences }): ReactElement { testID={`sidebar-row-item-${item}`} /> ))} + {TRAILING_CHOICES.map((choice) => ( , + [], + ); + return ( + + {t("sidebar.display.show.checks")} + + ); +} + function HostFilterPage({ preferences, hosts, diff --git a/packages/app/src/components/sidebar/display-preferences/model.ts b/packages/app/src/components/sidebar/display-preferences/model.ts index 9644e15b3..a56077487 100644 --- a/packages/app/src/components/sidebar/display-preferences/model.ts +++ b/packages/app/src/components/sidebar/display-preferences/model.ts @@ -5,6 +5,7 @@ import { type WorkspaceTitleSource, } from "@/hooks/use-settings"; import { useSidebarViewStore, type SidebarGroupMode } from "@/stores/sidebar-view-store"; +import { DEFAULT_SIDEBAR_CHECKS_DISPLAY, type SidebarChecksDisplay } from "./checks-display"; import { DEFAULT_SIDEBAR_ROW_ITEMS, type SidebarRowItem, type SidebarRowItems } from "./row-items"; /** The trailing slot holds one thing, so these are a choice rather than toggles. */ @@ -17,6 +18,8 @@ export interface SidebarDisplayPreferences { setTitleSource: (source: WorkspaceTitleSource) => void; rowItems: SidebarRowItems; toggleRowItem: (item: SidebarRowItem) => void; + checksDisplay: SidebarChecksDisplay; + setChecksDisplay: (display: SidebarChecksDisplay) => void; trailing: SidebarWorkspaceTrailing; /** Picking the choice that is already showing clears the slot. */ toggleTrailing: (choice: SidebarTrailingChoice) => void; @@ -41,7 +44,12 @@ export function useSidebarDisplayPreferences(): SidebarDisplayPreferences { const clearHostFilters = useSidebarViewStore((state) => state.clearHostFilters); const { - settings: { workspaceTitleSource, sidebarWorkspaceTrailing, sidebarRowItems }, + settings: { + workspaceTitleSource, + sidebarWorkspaceTrailing, + sidebarRowItems, + sidebarChecksDisplay, + }, updateSettings, } = useAppSettings(); @@ -61,6 +69,13 @@ export function useSidebarDisplayPreferences(): SidebarDisplayPreferences { [updateSettings, sidebarRowItems], ); + const setChecksDisplay = useCallback( + (display: SidebarChecksDisplay) => { + void updateSettings({ sidebarChecksDisplay: display }); + }, + [updateSettings], + ); + const toggleTrailing = useCallback( (choice: SidebarTrailingChoice) => { void updateSettings({ @@ -78,6 +93,8 @@ export function useSidebarDisplayPreferences(): SidebarDisplayPreferences { setTitleSource, rowItems: sidebarRowItems, toggleRowItem, + checksDisplay: sidebarChecksDisplay, + setChecksDisplay, trailing: sidebarWorkspaceTrailing, toggleTrailing, hostFilters, @@ -91,6 +108,8 @@ export function useSidebarDisplayPreferences(): SidebarDisplayPreferences { setTitleSource, sidebarRowItems, toggleRowItem, + sidebarChecksDisplay, + setChecksDisplay, sidebarWorkspaceTrailing, toggleTrailing, hostFilters, @@ -110,3 +129,24 @@ export function useSidebarRowItems(): SidebarRowItems { } = useAppSettings(); return sidebarRowItems ?? DEFAULT_SIDEBAR_ROW_ITEMS; } + +/** + * Everything the line under a workspace title needs to know, in one read. The two settings are + * answered together by `selectMetaRowItems`, so asking for them separately would only mean two + * subscriptions per row for one decision. + */ +export function useSidebarMetaPreferences(): { + rowItems: SidebarRowItems; + checksDisplay: SidebarChecksDisplay; +} { + const { + settings: { sidebarRowItems, sidebarChecksDisplay }, + } = useAppSettings(); + return useMemo( + () => ({ + rowItems: sidebarRowItems ?? DEFAULT_SIDEBAR_ROW_ITEMS, + checksDisplay: sidebarChecksDisplay ?? DEFAULT_SIDEBAR_CHECKS_DISPLAY, + }), + [sidebarRowItems, sidebarChecksDisplay], + ); +} diff --git a/packages/app/src/components/sidebar/display-preferences/row-items.test.ts b/packages/app/src/components/sidebar/display-preferences/row-items.test.ts index 6c2933457..c83c4a705 100644 --- a/packages/app/src/components/sidebar/display-preferences/row-items.test.ts +++ b/packages/app/src/components/sidebar/display-preferences/row-items.test.ts @@ -1,5 +1,10 @@ import { describe, expect, it } from "vitest"; -import { DEFAULT_SIDEBAR_ROW_ITEMS, parseSidebarRowItems, SIDEBAR_ROW_ITEMS } from "./row-items"; +import { + DEFAULT_SIDEBAR_ROW_ITEMS, + isChecksHiddenByLegacyRowItem, + parseSidebarRowItems, + SIDEBAR_ROW_ITEMS, +} from "./row-items"; describe("parseSidebarRowItems", () => { it("shows everything by default", () => { @@ -9,9 +14,9 @@ describe("parseSidebarRowItems", () => { }); it("applies stored overrides", () => { - expect(parseSidebarRowItems({ checks: false })).toEqual({ + expect(parseSidebarRowItems({ services: false })).toEqual({ ...DEFAULT_SIDEBAR_ROW_ITEMS, - checks: false, + services: false, }); }); @@ -25,20 +30,47 @@ describe("parseSidebarRowItems", () => { it("ignores unknown keys and non-boolean values", () => { expect( - parseSidebarRowItems({ checks: "yes", nonsense: false, scripts: null, host: false }), + parseSidebarRowItems({ checks: false, nonsense: false, services: null, host: false }), ).toEqual({ ...DEFAULT_SIDEBAR_ROW_ITEMS, host: false }); }); - it.each([[null], [undefined], ["diff"], [42], [["checks"]]])( + it.each([[null], [undefined], ["diff"], [42], [["services"]]])( "falls back to defaults for %s", (value) => { expect(parseSidebarRowItems(value)).toEqual(DEFAULT_SIDEBAR_ROW_ITEMS); }, ); + it("carries a switched-off scripts item over to services", () => { + // COMPAT(sidebarRowItemsScripts): the item narrowed from scripts to services in v0.3.0. + expect(parseSidebarRowItems({ scripts: false })).toEqual({ + ...DEFAULT_SIDEBAR_ROW_ITEMS, + services: false, + }); + }); + + it("lets a stored services value win over the scripts item it replaced", () => { + expect(parseSidebarRowItems({ scripts: false, services: true })).toEqual( + DEFAULT_SIDEBAR_ROW_ITEMS, + ); + }); + it("does not hand back the shared default object", () => { - const parsed = parseSidebarRowItems({ checks: false }); + const parsed = parseSidebarRowItems({ services: false }); expect(parsed).not.toBe(DEFAULT_SIDEBAR_ROW_ITEMS); - expect(DEFAULT_SIDEBAR_ROW_ITEMS.checks).toBe(true); + expect(DEFAULT_SIDEBAR_ROW_ITEMS.services).toBe(true); }); }); + +describe("isChecksHiddenByLegacyRowItem", () => { + it("reports the one stored shape that means checks were switched off", () => { + expect(isChecksHiddenByLegacyRowItem({ checks: false })).toBe(true); + }); + + it.each([[{ checks: true }], [{ services: false }], [{}], [null], ["checks"], [["checks"]]])( + "leaves %s to the new setting's default", + (value) => { + expect(isChecksHiddenByLegacyRowItem(value)).toBe(false); + }, + ); +}); diff --git a/packages/app/src/components/sidebar/display-preferences/row-items.ts b/packages/app/src/components/sidebar/display-preferences/row-items.ts index 313dbda18..3ef1d04aa 100644 --- a/packages/app/src/components/sidebar/display-preferences/row-items.ts +++ b/packages/app/src/components/sidebar/display-preferences/row-items.ts @@ -4,12 +4,15 @@ * Each item is independent — this is not a mode. Turning one off removes it from every row and * gives the space back to the title. * + * CI is not here: it has three answers rather than two, so it is its own setting in + * `checks-display.ts`. + * * Pure on purpose: `hooks/use-settings/storage.ts` validates the persisted value through * `parseSidebarRowItems` rather than growing its own checks, so this module stays the one place * that knows what an item is. */ -export const SIDEBAR_ROW_ITEMS = ["host", "changeRequest", "checks", "scripts"] as const; +export const SIDEBAR_ROW_ITEMS = ["host", "changeRequest", "services"] as const; export type SidebarRowItem = (typeof SIDEBAR_ROW_ITEMS)[number]; @@ -23,10 +26,21 @@ export type SidebarRowItems = Record; export const DEFAULT_SIDEBAR_ROW_ITEMS: SidebarRowItems = { host: true, changeRequest: true, - checks: true, - scripts: true, + services: true, }; +/** + * COMPAT(sidebarRowItemsChecks): CI was a boolean row item until v0.3.0, when it became a + * display mode of its own. A stored `false` is someone who switched checks off, and they must + * not come back as icon-and-text on upgrade. Remove after 2027-08-05. + */ +export function isChecksHiddenByLegacyRowItem(value: unknown): boolean { + if (typeof value !== "object" || value === null || Array.isArray(value)) { + return false; + } + return (value as Record).checks === false; +} + function isSidebarRowItem(value: string): value is SidebarRowItem { return (SIDEBAR_ROW_ITEMS as readonly string[]).includes(value); } @@ -38,6 +52,11 @@ export function parseSidebarRowItems(value: unknown): SidebarRowItems { const stored = value as Record; const result = { ...DEFAULT_SIDEBAR_ROW_ITEMS }; + // COMPAT(sidebarRowItemsScripts): the item was called "scripts" until v0.3.0, when it narrowed + // to services only. A stored `false` is someone who switched it off. Remove after 2027-08-05. + if (stored.scripts === false) { + result.services = false; + } for (const [key, entry] of Object.entries(stored)) { if (isSidebarRowItem(key) && typeof entry === "boolean") { result[key] = entry; diff --git a/packages/app/src/components/sidebar/sidebar-group-toggle-row.tsx b/packages/app/src/components/sidebar/sidebar-group-toggle-row.tsx index d6b0c6e04..5b458ae71 100644 --- a/packages/app/src/components/sidebar/sidebar-group-toggle-row.tsx +++ b/packages/app/src/components/sidebar/sidebar-group-toggle-row.tsx @@ -4,6 +4,7 @@ import { Pressable, Text, View, type PressableStateCallbackType } from "react-na import { ChevronDown, ChevronUp } from "lucide-react-native"; import { StyleSheet, withUnistyles } from "react-native-unistyles"; import { isWeb } from "@/constants/platform"; +import { sidebarWorkspaceRowStyles } from "@/components/sidebar/sidebar-workspace-row-content"; import type { Theme } from "@/styles/theme"; const foregroundMutedColorMapping = (theme: Theme) => ({ @@ -15,13 +16,25 @@ const foregroundColorMapping = (theme: Theme) => ({ const ThemedChevronDown = withUnistyles(ChevronDown); const ThemedChevronUp = withUnistyles(ChevronUp); +/** + * The row that ends a truncated group. It is a workspace row that happens to say "Show more", so + * it takes the workspace row's geometry — height, padding, radius, and both fills — rather than a + * set of its own. Sitting under a column of rows, anything it does differently reads as a mistake + * rather than as a distinction. + * + * `indented` because the two groupings disagree: status rows sit on their header's label rail and + * project rows sit flush, so the caller says which list this row is ending. The indent itself is + * the workspace row's, imported rather than re-derived. + */ export function SidebarGroupToggleRow({ expanded, onPress, + indented = false, testID, }: { expanded: boolean; onPress: () => void; + indented?: boolean; testID: string; }) { const { t } = useTranslation(); @@ -31,9 +44,11 @@ export function SidebarGroupToggleRow({ const rowStyle = useCallback( ({ hovered = false, pressed }: PressableStateCallbackType & { hovered?: boolean }) => [ styles.row, - (Boolean(hovered) || pressed) && styles.rowHovered, + indented && sidebarWorkspaceRowStyles.rowIndented, + hovered && !pressed && styles.rowHovered, + pressed && styles.rowPressed, ], - [], + [indented], ); return ( @@ -69,13 +84,14 @@ export function SidebarGroupToggleRow({ } const styles = StyleSheet.create((theme) => ({ + // Kept in step with `workspaceRow` in sidebar-workspace-list.tsx and sidebar-status-list.tsx. row: { - minHeight: 32, - marginLeft: theme.spacing[6], - marginRight: theme.spacing[1], - paddingVertical: theme.spacing[1], - paddingHorizontal: theme.spacing[2], - borderRadius: theme.borderRadius.md, + minHeight: 36, + marginBottom: theme.spacing[0.5], + paddingVertical: theme.spacing[2], + paddingLeft: theme.spacing[2], + paddingRight: theme.spacing[3], + borderRadius: theme.borderRadius.lg, flexDirection: "row", alignItems: "center", gap: theme.spacing[2], @@ -84,9 +100,14 @@ const styles = StyleSheet.create((theme) => ({ rowHovered: { backgroundColor: theme.colors.surfaceSidebarHover, }, + rowPressed: { + backgroundColor: theme.colors.surface2, + }, + // The width of a workspace row's status slot, so the label lands on the same rail as the + // titles above it rather than two points to their left. iconSlot: { - width: theme.iconSize.sm, - height: theme.iconSize.sm, + width: theme.iconSize.md, + height: theme.iconSize.md, alignItems: "center", justifyContent: "center", flexShrink: 0, diff --git a/packages/app/src/components/sidebar/sidebar-status-list.tsx b/packages/app/src/components/sidebar/sidebar-status-list.tsx index a89e6f130..f8a9acbed 100644 --- a/packages/app/src/components/sidebar/sidebar-status-list.tsx +++ b/packages/app/src/components/sidebar/sidebar-status-list.tsx @@ -44,7 +44,7 @@ import { import { useOpenKebabMenuVisibility } from "@/components/sidebar/use-open-kebab-menu-visibility"; import { getSidebarRowBackdrop } from "@/components/sidebar/sidebar-row-backdrop"; import { getStatusDotColor } from "@/utils/status-dot-color"; -import { selectWorkspaceScriptSummary } from "@/components/sidebar/workspace-meta-row"; +import { selectWorkspaceServiceSummary } from "@/components/sidebar/workspace-meta-row"; import { SidebarWorkspaceTrailingContent, useSidebarWorkspaceTrailing, @@ -270,7 +270,7 @@ function StatusGroupRows({ } = useLimitedSidebarGroup(group.rows); return ( - + {!collapsed ? ( ) : null} @@ -689,7 +690,7 @@ function StatusWorkspaceRowInner({ const trailing = useSidebarWorkspaceTrailing(); const isDesktop = !isTouchPlatform; - const scriptSummary = isDesktop ? selectWorkspaceScriptSummary(workspace.scripts) : null; + const serviceSummary = isDesktop ? selectWorkspaceServiceSummary(workspace.scripts) : null; const accessibilityState = useMemo(() => ({ selected }), [selected]); @@ -724,7 +725,7 @@ function StatusWorkspaceRowInner({ workspace={workspace} leadingProjectName={projectName} hostBadgeLabel={hostBadge?.label} - scriptSummary={scriptSummary} + serviceSummary={serviceSummary} workspaceKey={workspace.workspaceKey} onCopyPath={onCopyPath} onCopyBranchName={onCopyBranchName} @@ -751,7 +752,7 @@ function StatusWorkspaceRowInner({ hostBadge={hostBadge} leadingProjectName={projectName} leadingProjectIconDataUri={projectIconDataUri} - scriptSummary={scriptSummary} + serviceSummary={serviceSummary} backdrop={getSidebarRowBackdrop({ selected, isHovered })} isHovered={isHovered} isLoading={isArchiving} @@ -888,8 +889,9 @@ const styles = StyleSheet.create((theme) => ({ pinnedSection: { marginBottom: theme.spacing[1], }, - statusGroupBlock: { - marginBottom: theme.spacing[1], + // Matches `projectBlockExpanded` in sidebar-workspace-list.tsx. See the note there. + statusGroupBlockExpanded: { + paddingBottom: theme.spacing[3], }, statusWorkspaceListContainer: {}, statusGroupRow: { @@ -944,7 +946,7 @@ const styles = StyleSheet.create((theme) => ({ }, workspaceRow: { minHeight: 36, - marginBottom: theme.spacing[1], + marginBottom: theme.spacing[0.5], paddingVertical: theme.spacing[2], paddingLeft: theme.spacing[2], paddingRight: theme.spacing[3], diff --git a/packages/app/src/components/sidebar/sidebar-workspace-menu.tsx b/packages/app/src/components/sidebar/sidebar-workspace-menu.tsx index c33fa51ca..332b23842 100644 --- a/packages/app/src/components/sidebar/sidebar-workspace-menu.tsx +++ b/packages/app/src/components/sidebar/sidebar-workspace-menu.tsx @@ -24,7 +24,10 @@ import { import { Shortcut } from "@/components/ui/shortcut"; import { OpenInFileManagerMenuItem } from "@/workspace/open-in-file-manager/menu-item"; import { resolveSidebarWorkspaceAccessibilityLabel } from "@/components/sidebar/sidebar-workspace-title"; -import type { WorkspaceScriptSummary } from "@/components/sidebar/workspace-meta-row"; +import { + workspaceServiceLabelKey, + type WorkspaceServiceSummary, +} from "@/components/sidebar/workspace-meta-row"; const foregroundColorMapping = (theme: Theme) => ({ color: theme.colors.foreground }); const foregroundMutedColorMapping = (theme: Theme) => ({ @@ -260,7 +263,7 @@ export function SidebarWorkspaceContextMenu({ workspace, leadingProjectName, hostBadgeLabel, - scriptSummary, + serviceSummary, workspaceKey, onCopyPath, onCopyBranchName, @@ -285,7 +288,7 @@ export function SidebarWorkspaceContextMenu({ workspace: SidebarWorkspaceEntry; leadingProjectName?: string | null; hostBadgeLabel?: string | null; - scriptSummary?: WorkspaceScriptSummary | null; + serviceSummary?: WorkspaceServiceSummary | null; highlightStyle: ComponentProps["highlightStyle"]; } >) { @@ -305,7 +308,9 @@ export function SidebarWorkspaceContextMenu({ leadingProjectName, hostBadgeLabel, pullRequestLabel, - scriptLabel: scriptSummary ? t("workspace.status.scriptsAvailable") : null, + serviceLabel: serviceSummary + ? t(workspaceServiceLabelKey(serviceSummary), { name: serviceSummary.name }) + : null, }); return ( diff --git a/packages/app/src/components/sidebar/sidebar-workspace-row-content.tsx b/packages/app/src/components/sidebar/sidebar-workspace-row-content.tsx index ae5555ee1..a07ecc205 100644 --- a/packages/app/src/components/sidebar/sidebar-workspace-row-content.tsx +++ b/packages/app/src/components/sidebar/sidebar-workspace-row-content.tsx @@ -8,7 +8,7 @@ import type { SurfaceBackdrop } from "@/styles/surface-backdrop"; import { PulsingStatusDot } from "@/components/sidebar/pulsing-status-dot"; import { WorkspaceMetaRow, - type WorkspaceScriptSummary, + type WorkspaceServiceSummary, } from "@/components/sidebar/workspace-meta-row"; import { WorkspaceHoverCard } from "@/components/workspace-hover-card"; import type { HostBadgeModel } from "@/hosts/appearance"; @@ -122,7 +122,7 @@ export const SidebarWorkspaceRowContent = memo(function SidebarWorkspaceRowConte hostBadge, leadingProjectName = null, leadingProjectIconDataUri = null, - scriptSummary = null, + serviceSummary = null, backdrop, isHovered, isLoading, @@ -137,7 +137,7 @@ export const SidebarWorkspaceRowContent = memo(function SidebarWorkspaceRowConte /** Hoisted rows use their project icon as the leading visual because no project row contains them. */ leadingProjectName?: string | null; leadingProjectIconDataUri?: string | null; - scriptSummary?: WorkspaceScriptSummary | null; + serviceSummary?: WorkspaceServiceSummary | null; /** The row's current background, so the project status badge can knock out of it. */ backdrop: SurfaceBackdrop; isHovered: boolean; @@ -193,7 +193,7 @@ export const SidebarWorkspaceRowContent = memo(function SidebarWorkspaceRowConte diff --git a/packages/app/src/components/sidebar/sidebar-workspace-row.tsx b/packages/app/src/components/sidebar/sidebar-workspace-row.tsx index 33aca3aa2..2b3a6e33c 100644 --- a/packages/app/src/components/sidebar/sidebar-workspace-row.tsx +++ b/packages/app/src/components/sidebar/sidebar-workspace-row.tsx @@ -34,7 +34,7 @@ import { } from "@/components/sidebar/sidebar-workspace-row-content"; import { useOpenKebabMenuVisibility } from "@/components/sidebar/use-open-kebab-menu-visibility"; import { getSidebarRowBackdrop } from "@/components/sidebar/sidebar-row-backdrop"; -import { selectWorkspaceScriptSummary } from "@/components/sidebar/workspace-meta-row"; +import { selectWorkspaceServiceSummary } from "@/components/sidebar/workspace-meta-row"; import { SidebarWorkspaceTrailingContent, useSidebarWorkspaceTrailing, @@ -291,7 +291,7 @@ function WorkspaceRowBody({ {({ isHovered, contextMenuOpen, onContextMenuOpenChange, hoverHandlers }) => { const isDesktop = !isTouchPlatform; - const scriptSummary = isDesktop ? selectWorkspaceScriptSummary(workspace.scripts) : null; + const serviceSummary = isDesktop ? selectWorkspaceServiceSummary(workspace.scripts) : null; const workspaceRowStyle = getWorkspaceRowStyle({ isDragging, selected, isHovered }); return ( { leadingProjectName: "Search project", hostBadgeLabel: "Build host", pullRequestLabel: "Pull request 42", - scriptLabel: "Scripts available", + serviceLabel: "Service web running", }); expect(label).toBe( - "Search project, fix/search, Build host, Pull request 42, Scripts available, Working", + "Search project, fix/search, Build host, Pull request 42, Service web running, Working", ); }); diff --git a/packages/app/src/components/sidebar/sidebar-workspace-title.ts b/packages/app/src/components/sidebar/sidebar-workspace-title.ts index e9634e021..955c9d263 100644 --- a/packages/app/src/components/sidebar/sidebar-workspace-title.ts +++ b/packages/app/src/components/sidebar/sidebar-workspace-title.ts @@ -18,14 +18,14 @@ export function resolveSidebarWorkspaceAccessibilityLabel(input: { leadingProjectName?: string | null; hostBadgeLabel?: string | null; pullRequestLabel?: string | null; - scriptLabel?: string | null; + serviceLabel?: string | null; }): string { return [ input.leadingProjectName, resolveSidebarWorkspacePrimaryLabel(input), input.hostBadgeLabel, input.pullRequestLabel, - input.scriptLabel, + input.serviceLabel, input.workspace.statusBucket === "done" ? null : STATUS_BUCKET_LABELS[input.workspace.statusBucket], diff --git a/packages/app/src/components/sidebar/workspace-meta-row/index.tsx b/packages/app/src/components/sidebar/workspace-meta-row/index.tsx index 436d048f2..e7d56ff02 100644 --- a/packages/app/src/components/sidebar/workspace-meta-row/index.tsx +++ b/packages/app/src/components/sidebar/workspace-meta-row/index.tsx @@ -8,21 +8,24 @@ import { GitPullRequest, GitPullRequestClosed, Globe, - SquareTerminal, } from "lucide-react-native"; import type { HostBadgeModel } from "@/hosts/appearance"; import { HostBadge, HOST_BADGE_ICON_SIZE } from "@/hosts/host-badge"; import type { PrHint } from "@/git/pr-hint"; import { getForgePresentation, normalizeForge } from "@/git/forge"; import { openExternalUrl } from "@/utils/open-external-url"; -import { useSidebarRowItems } from "@/components/sidebar/display-preferences/model"; +import { useSidebarMetaPreferences } from "@/components/sidebar/display-preferences/model"; import type { Theme } from "@/styles/theme"; import { CheckIndicator } from "./check-indicator"; import type { CheckSummary, CheckSummaryState } from "./check-summary"; import { selectMetaRowItems, type MetaRowItem } from "./meta-items"; -import type { WorkspaceScriptSummary } from "./script-summary"; +import { workspaceServiceLabelKey, type WorkspaceServiceSummary } from "./service-summary"; -export { selectWorkspaceScriptSummary, type WorkspaceScriptSummary } from "./script-summary"; +export { + selectWorkspaceServiceSummary, + workspaceServiceLabelKey, + type WorkspaceServiceSummary, +} from "./service-summary"; /** * One size for every glyph on the line. The items are peers — host, change request, CI, @@ -36,7 +39,6 @@ const ThemedGitPullRequest = withUnistyles(GitPullRequest); const ThemedGitMerge = withUnistyles(GitMerge); const ThemedGitPullRequestClosed = withUnistyles(GitPullRequestClosed); const ThemedGlobe = withUnistyles(Globe); -const ThemedSquareTerminal = withUnistyles(SquareTerminal); const foregroundMapping = (theme: Theme) => ({ color: theme.colors.foreground }); const mergedMapping = (theme: Theme) => ({ color: theme.colors.statusMerged }); @@ -55,18 +57,19 @@ const dangerMapping = (theme: Theme) => ({ color: theme.colors.statusDanger }); export function WorkspaceMetaRow({ hostBadge, prHint, - scriptSummary, + serviceSummary, }: { hostBadge: HostBadgeModel | null; prHint: PrHint | null; - scriptSummary: WorkspaceScriptSummary | null; + serviceSummary: WorkspaceServiceSummary | null; }) { - const visible = useSidebarRowItems(); + const { rowItems, checksDisplay } = useSidebarMetaPreferences(); const items = selectMetaRowItems({ hasHostBadge: hostBadge !== null, prHint, - scriptSummary, - visible, + serviceSummary, + visible: rowItems, + checksDisplay, }); if (items.length === 0) return null; @@ -97,9 +100,9 @@ function MetaItemNode({ return ; } if (item.kind === "checks") { - return ; + return ; } - return ; + return ; } /** @@ -158,15 +161,19 @@ function PullRequestItem({ hint }: { hint: PrHint }) { } /** - * Every state says its own name. A tick on its own has no subject — the reader has to already - * know the glyph is about CI to read it at all, and sitting after a host and a change request it - * reads as something left over. One short word costs less than that ambiguity. + * By default every state says its own name. A tick on its own has no subject — the reader has to + * already know the glyph is about CI to read it at all, and sitting after a host and a change + * request it reads as something left over. One short word costs less than that ambiguity, which + * is why icon-and-text is the default and dropping the word is something you ask for. * * The words are the outcome, not the noun: "passed" against "failed", both naming how a finished * run ended, "running" for one still deciding. Repeating the subject on every healthy row — * "checks passed" — is the noise worth avoiding; naming the outcome is not. + * + * The accessible name is on the wrapper either way, so turning the word off shrinks the row + * without taking the state away from a screen reader. */ -function ChecksItem({ summary }: { summary: CheckSummary }) { +function ChecksItem({ summary, label }: { summary: CheckSummary; label: boolean }) { const { t } = useTranslation(); return ( - - {t(CHECK_STATE_LABEL_KEYS[summary.state])} - + {label ? ( + + {t(CHECK_STATE_LABEL_KEYS[summary.state])} + + ) : null} ); } @@ -194,20 +203,27 @@ const CHECK_STATE_ACCESSIBLE_KEYS = { running: "workspace.git.pr.checksSummary.runningAccessible", } as const; -function ScriptItem({ summary }: { summary: WorkspaceScriptSummary }) { +/** + * A running service, named. It is the one item on the line whose text is arbitrary — everything + * else is a number, a short word, or a host label the user already picked — so it is also the + * only one allowed to shrink, and it truncates rather than pushing the line past the row. + * + * A failed health check turns the whole item danger, glyph and name together. Colouring only the + * glyph would leave the name reading as fine, and the name is the part you look at. + */ +function ServiceItem({ summary }: { summary: WorkspaceServiceSummary }) { const { t } = useTranslation(); + const unhealthy = summary.health === "unhealthy"; return ( - {summary.kind === "service" ? ( - - ) : ( - - )} - {summary.port !== null ? :{summary.port} : null} + + + {summary.name} + ); } @@ -260,11 +276,25 @@ const styles = StyleSheet.create((theme) => ({ lineHeight: 16, flexShrink: 0, }, - scriptPort: { + // The one item that gives way when the line runs out of room — see `ServiceItem`. + serviceItem: { + flexDirection: "row", + alignItems: "center", + gap: 3, + minWidth: 0, + flexShrink: 1, + }, + serviceName: { color: theme.colors.statusSuccess, fontSize: theme.fontSize.xs, lineHeight: 16, - flexShrink: 0, + flexShrink: 1, + }, + serviceNameUnhealthy: { + color: theme.colors.statusDanger, + fontSize: theme.fontSize.xs, + lineHeight: 16, + flexShrink: 1, }, prText: { color: theme.colors.foregroundMuted, diff --git a/packages/app/src/components/sidebar/workspace-meta-row/meta-items.test.ts b/packages/app/src/components/sidebar/workspace-meta-row/meta-items.test.ts index 4b8ab5a7c..fc3c424fa 100644 --- a/packages/app/src/components/sidebar/workspace-meta-row/meta-items.test.ts +++ b/packages/app/src/components/sidebar/workspace-meta-row/meta-items.test.ts @@ -1,8 +1,9 @@ import { describe, expect, it } from "vitest"; import type { PrHint } from "@/git/pr-hint"; +import { DEFAULT_SIDEBAR_CHECKS_DISPLAY } from "@/components/sidebar/display-preferences/checks-display"; import { DEFAULT_SIDEBAR_ROW_ITEMS } from "@/components/sidebar/display-preferences/row-items"; import { selectMetaRowItems } from "./meta-items"; -import type { WorkspaceScriptSummary } from "./script-summary"; +import type { WorkspaceServiceSummary } from "./service-summary"; const PR_HINT: PrHint = { url: "https://github.com/acme/app/pull/7", @@ -12,14 +13,15 @@ const PR_HINT: PrHint = { checksStatus: "success", }; -const SCRIPT: WorkspaceScriptSummary = { kind: "service", port: 3000 }; +const SERVICE: WorkspaceServiceSummary = { name: "web", health: null }; function select(overrides: Partial[0]> = {}) { return selectMetaRowItems({ hasHostBadge: true, prHint: PR_HINT, - scriptSummary: SCRIPT, + serviceSummary: SERVICE, visible: DEFAULT_SIDEBAR_ROW_ITEMS, + checksDisplay: DEFAULT_SIDEBAR_CHECKS_DISPLAY, ...overrides, }); } @@ -28,37 +30,39 @@ const kinds = (items: ReturnType) => items.map((item) describe("selectMetaRowItems", () => { it("reads identity, then the change, then its state, then what is running", () => { - expect(kinds(select())).toEqual(["host", "changeRequest", "checks", "scripts"]); + expect(kinds(select())).toEqual(["host", "changeRequest", "checks", "services"]); }); it("omits what the workspace does not have", () => { - expect(kinds(select({ hasHostBadge: false, prHint: null, scriptSummary: null }))).toEqual([]); + expect(kinds(select({ hasHostBadge: false, prHint: null, serviceSummary: null }))).toEqual([]); }); it.each([ - ["changeRequest", ["host", "checks", "scripts"]], - ["checks", ["host", "changeRequest", "scripts"]], - ["scripts", ["host", "changeRequest", "checks"]], + ["changeRequest", ["host", "checks", "services"]], + ["services", ["host", "changeRequest", "checks"]], ] as const)("drops %s and only %s when it is switched off", (item, expected) => { expect(kinds(select({ visible: { ...DEFAULT_SIDEBAR_ROW_ITEMS, [item]: false } }))).toEqual( expected, ); }); + it("drops checks and only checks when they are hidden", () => { + expect(kinds(select({ checksDisplay: "none" }))).toEqual(["host", "changeRequest", "services"]); + }); + it("keeps checks when the change request is hidden", () => { - // Each toggle answers for itself. A ticked "checks" that drew nothing because a different + // Each control answers for itself. A checks setting that drew nothing because a different // switch was off would be lying about its own state. - const items = select({ - visible: { ...DEFAULT_SIDEBAR_ROW_ITEMS, changeRequest: false, checks: true }, - }); - expect(kinds(items)).toEqual(["host", "checks", "scripts"]); + const items = select({ visible: { ...DEFAULT_SIDEBAR_ROW_ITEMS, changeRequest: false } }); + expect(kinds(items)).toEqual(["host", "checks", "services"]); }); it("draws nothing for checks when both are off", () => { const items = select({ - visible: { ...DEFAULT_SIDEBAR_ROW_ITEMS, changeRequest: false, checks: false }, + visible: { ...DEFAULT_SIDEBAR_ROW_ITEMS, changeRequest: false }, + checksDisplay: "none", }); - expect(kinds(items)).toEqual(["host", "scripts"]); + expect(kinds(items)).toEqual(["host", "services"]); }); it("carries the resolved check summary rather than the raw hint", () => { @@ -66,11 +70,21 @@ describe("selectMetaRowItems", () => { expect(checks).toEqual({ kind: "checks", summary: { state: "passed", completed: 1, total: 1 }, + label: true, + }); + }); + + it("keeps the check summary but drops its word when only the icon is wanted", () => { + const checks = select({ checksDisplay: "icon" }).find((item) => item.kind === "checks"); + expect(checks).toEqual({ + kind: "checks", + summary: { state: "passed", completed: 1, total: 1 }, + label: false, }); }); it("keeps a change request whose forge reports no checks", () => { const items = select({ prHint: { ...PR_HINT, checksStatus: undefined } }); - expect(kinds(items)).toEqual(["host", "changeRequest", "scripts"]); + expect(kinds(items)).toEqual(["host", "changeRequest", "services"]); }); }); diff --git a/packages/app/src/components/sidebar/workspace-meta-row/meta-items.ts b/packages/app/src/components/sidebar/workspace-meta-row/meta-items.ts index fda162c2d..98a52fcfc 100644 --- a/packages/app/src/components/sidebar/workspace-meta-row/meta-items.ts +++ b/packages/app/src/components/sidebar/workspace-meta-row/meta-items.ts @@ -1,7 +1,8 @@ import type { PrHint } from "@/git/pr-hint"; +import type { SidebarChecksDisplay } from "@/components/sidebar/display-preferences/checks-display"; import type { SidebarRowItems } from "@/components/sidebar/display-preferences/row-items"; import { selectCheckSummary, type CheckSummary } from "./check-summary"; -import type { WorkspaceScriptSummary } from "./script-summary"; +import type { WorkspaceServiceSummary } from "./service-summary"; /** * What ends up on the line under a workspace title, in the order it is read: where the @@ -11,8 +12,8 @@ import type { WorkspaceScriptSummary } from "./script-summary"; export type MetaRowItem = | { kind: "host" } | { kind: "changeRequest"; hint: PrHint } - | { kind: "checks"; summary: CheckSummary } - | { kind: "scripts"; summary: WorkspaceScriptSummary }; + | { kind: "checks"; summary: CheckSummary; label: boolean } + | { kind: "services"; summary: WorkspaceServiceSummary }; /** * Which peers a row should draw, given what it knows and what the user left switched on. @@ -27,10 +28,11 @@ export type MetaRowItem = export function selectMetaRowItems(input: { hasHostBadge: boolean; prHint: PrHint | null; - scriptSummary: WorkspaceScriptSummary | null; + serviceSummary: WorkspaceServiceSummary | null; visible: SidebarRowItems; + checksDisplay: SidebarChecksDisplay; }): MetaRowItem[] { - const { hasHostBadge, prHint, scriptSummary, visible } = input; + const { hasHostBadge, prHint, serviceSummary, visible, checksDisplay } = input; const items: MetaRowItem[] = []; if (hasHostBadge) { @@ -41,18 +43,18 @@ export function selectMetaRowItems(input: { } // Independent of the change request, even though checks are read off one. Tying them together - // meant the checks toggle could sit ticked while nothing was drawn, which is a switch that lies - // about its own state. Showing checks without the change request beside them is the stranger - // combination, but it is the one you asked for and it is what you get. - if (visible.checks) { + // meant the checks setting could sit on a value while nothing was drawn, which is a control that + // lies about its own state. Showing checks without the change request beside them is the + // stranger combination, but it is the one you asked for and it is what you get. + if (checksDisplay !== "none") { const summary = selectCheckSummary(prHint); if (summary) { - items.push({ kind: "checks", summary }); + items.push({ kind: "checks", summary, label: checksDisplay === "iconAndText" }); } } - if (scriptSummary && visible.scripts) { - items.push({ kind: "scripts", summary: scriptSummary }); + if (serviceSummary && visible.services) { + items.push({ kind: "services", summary: serviceSummary }); } return items; diff --git a/packages/app/src/components/sidebar/workspace-meta-row/script-summary.test.ts b/packages/app/src/components/sidebar/workspace-meta-row/script-summary.test.ts deleted file mode 100644 index 38ec60c4c..000000000 --- a/packages/app/src/components/sidebar/workspace-meta-row/script-summary.test.ts +++ /dev/null @@ -1,66 +0,0 @@ -import { describe, expect, it } from "vitest"; -import type { SidebarWorkspaceEntry } from "@/hooks/use-sidebar-workspaces-list"; -import { selectWorkspaceScriptSummary } from "./script-summary"; - -type Script = SidebarWorkspaceEntry["scripts"][number]; - -function script(overrides: Partial