-
Notifications
You must be signed in to change notification settings - Fork 325
Make environment host labels responsive #2056
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,9 +16,9 @@ const CHECKOUT_CHIP_BUTTON_CLASS_NAME = `${CHECKOUT_CHIP_BASE_CLASS_NAME} cursor | |
| export interface ThreadEnvironmentSummaryProps { | ||
| /** Display name of the thread's project, shown alongside the environment. */ | ||
| projectName?: string; | ||
| /** Full mode label used for the title (e.g. "Working locally" / "Worktree"). */ | ||
| /** Full mode label used on larger prompt boxes and in the title. */ | ||
| environmentLabel?: string; | ||
| /** Visible label used in the promptbox footer. */ | ||
| /** Short label used when the promptbox switches to its compact layout. */ | ||
| environmentCompactLabel?: string; | ||
| /** Icon for the environment (e.g. monitor / git branch). */ | ||
| environmentIcon?: IconName; | ||
|
|
@@ -36,7 +36,8 @@ export interface ThreadEnvironmentSummaryProps { | |
| * Read-only — environment editing happens elsewhere. | ||
| * | ||
| * Responsive behavior: | ||
| * - The visible environment label always uses the compact display string. | ||
| * - The full environment label is replaced by the compact display string in | ||
| * narrow promptbox shells. | ||
| * - The summary can shrink inside the follow-up strip so permission/context | ||
| * controls stay pinned and text truncates instead of wrapping. | ||
| * - Branch chip hides only in very narrow promptbox shells and truncates | ||
|
|
@@ -55,8 +56,6 @@ export const ThreadEnvironmentSummary = memo(function ThreadEnvironmentSummary({ | |
| } | ||
|
|
||
| const checkoutCopyValue = environmentCheckout?.copyValue ?? null; | ||
| const visibleEnvironmentLabel = environmentCompactLabel ?? environmentLabel; | ||
|
|
||
| return ( | ||
| <div className="flex min-w-0 max-w-full items-center gap-2 pr-1.5"> | ||
| {projectName ? ( | ||
|
|
@@ -72,8 +71,8 @@ export const ThreadEnvironmentSummary = memo(function ThreadEnvironmentSummary({ | |
| ) : null} | ||
| <OptionDisplay | ||
| label="Environment" | ||
| value={visibleEnvironmentLabel} | ||
| compactValue={visibleEnvironmentLabel} | ||
| value={environmentLabel} | ||
| compactValue={environmentCompactLabel} | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨
The container CSS hides the full span below 34 rem. The environment label then becomes empty. Restore |
||
| leading={ | ||
| environmentIcon ? ( | ||
| <Icon name={environmentIcon} className="size-4 shrink-0" /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import type { Host } from "@bb/domain"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import type { ThreadEnvironmentSelection } from "@/data/compose"; | ||
| import { describeEnvironmentSelection } from "./environment-picker-model"; | ||
|
|
||
| const host: Host = { | ||
| id: "host_primary", | ||
| name: "MacBook Pro", | ||
| type: "persistent", | ||
| status: "connected", | ||
| lastSeenAt: null, | ||
| maxPermissionMode: "full", | ||
| lastRejectedProtocolVersion: null, | ||
| createdAt: 0, | ||
| updatedAt: 0, | ||
| }; | ||
|
|
||
| const worktreeSelection: ThreadEnvironmentSelection = { | ||
| type: "host", | ||
| hostId: host.id, | ||
| workspace: { type: "managed-worktree", baseBranch: null }, | ||
| }; | ||
|
|
||
| describe("describeEnvironmentSelection", () => { | ||
| it("omits the machine name from the mobile environment label", () => { | ||
| expect(describeEnvironmentSelection(worktreeSelection, host, []).label).toBe( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🚨
Please format this test. |
||
| "New worktree", | ||
| ); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,56 @@ | ||
| import type { Host } from "@bb/domain"; | ||
| import type { | ||
| ReuseEnvironmentOption, | ||
| ThreadEnvironmentSelection, | ||
| } from "@/data/compose"; | ||
|
|
||
| export interface EnvironmentSelectionSummary { | ||
| label: string; | ||
| icon: "Laptop" | "FolderGit" | "Folder"; | ||
| tone: "default" | "warning"; | ||
| } | ||
|
|
||
| export function describeEnvironmentSelection( | ||
| value: ThreadEnvironmentSelection, | ||
| host: Host | null, | ||
| reuseOptions: readonly ReuseEnvironmentOption[], | ||
| ): EnvironmentSelectionSummary { | ||
| switch (value.type) { | ||
| case "project-default": | ||
| return { label: "Project default", icon: "Laptop", tone: "default" }; | ||
| case "reuse": { | ||
| const option = reuseOptions.find( | ||
| (candidate) => candidate.environmentId === value.environmentId, | ||
| ); | ||
| const name = option?.name ?? option?.branchName; | ||
| return { | ||
| label: name ? `Reuse ${name}` : "Reuse worktree", | ||
| icon: "FolderGit", | ||
| tone: "default", | ||
| }; | ||
| } | ||
| case "host": { | ||
| const offline = host !== null && host.status !== "connected"; | ||
| if (value.workspace.type === "managed-worktree") { | ||
| return { | ||
| label: "New worktree", | ||
| icon: "FolderGit", | ||
| tone: offline ? "warning" : "default", | ||
| }; | ||
| } | ||
| if (value.workspace.type === "personal") { | ||
| return { | ||
| label: "Personal workspace", | ||
| icon: "Laptop", | ||
| tone: offline ? "warning" : "default", | ||
| }; | ||
| } | ||
| const custom = value.workspace.path; | ||
| return { | ||
| label: custom ?? "Work in checkout", | ||
| icon: "Folder", | ||
| tone: offline ? "warning" : "default", | ||
| }; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚨
slopcop/review— The wide follow-up label truncates.valuenow receives a host-prefixed label. The chip still has a 10-rem maximum width.In the two-host browser test, the text needed 189 pixels but received 130 pixels. The compact label displayed correctly at 390 pixels.
Please allow more width or preserve the mode when the host name truncates. The new unit test checks text nodes only.