diff --git a/.agents/skills/test-t3-mobile/SKILL.md b/.agents/skills/test-t3-mobile/SKILL.md index 611b25e34cdb..587b9b120b8b 100644 --- a/.agents/skills/test-t3-mobile/SKILL.md +++ b/.agents/skills/test-t3-mobile/SKILL.md @@ -125,31 +125,29 @@ Do not start, stop, erase, or reconfigure an emulator owned by another task. Tra ## Pair each client once -Issue a fresh credential against the running backend's exact base directory: +Use the bundled helper from the repository root. It issues a fresh credential against the running backend's exact base directory, opens the existing Add Environment route with the credential in an encoded query parameter, and asks that route to connect once: ```bash -MARCODE_PORT= node apps/server/src/bin.ts auth pairing create \ - --base-dir \ - --base-url \ - --ttl 15m \ - --label agent-mobile- +.agents/skills/test-t3-mobile/scripts/pair-client.sh \ + ios + +.agents/skills/test-t3-mobile/scripts/pair-client.sh \ + android ``` -In PowerShell, set `$env:MARCODE_PORT = ""` first and run the `node ... auth pairing create` command without the leading assignment. +Run only the command for the selected platform. The helper uses `http://127.0.0.1:` for iOS and `http://10.0.2.2:` for Android. Pass a fifth argument only when testing a non-development URL scheme. -If the visible Add Environment action is not exposed as a semantic target, open the app's registered route instead of guessing coordinates: +The helper opens this registered route: -```bash -xcrun simctl openurl 't3code-dev://connections/new' -adb -s shell am start -W \ - -a android.intent.action.VIEW \ - -d 't3code-dev://connections/new' \ - com.t3tools.marcode.dev +```text +t3code-dev://connections/new?pairingUrl=&autoConnect=1 ``` -Run only the command for the selected platform. +The Add Environment route owns the behavior: `pairingUrl` prefills its normal host and token inputs, while `autoConnect=1` submits once in development builds and returns to Home after success. Without `autoConnect`, the same route only prefills the form for manual inspection. + +Do not enter pairing hosts or tokens through simulator keyboard automation. Xcode's semantic typer sends HID-style key events through the simulator's active keyboard state, which can corrupt uppercase tokens and punctuation even when the host Mac uses a U.S. input source. The one-shot route is the deterministic pairing path. Use the visible form only as a fallback, and paste credentials rather than typing them character by character. -In T3 Code Dev, open Add Environment and enter the complete `` and newly printed `Token`. Verify the expected seeded projects appear before exercising the affected flow. +Verify the expected seeded projects appear before exercising the affected flow. Pairing credentials are secret, short-lived, and single-use. Create a different credential for every simulator, emulator, physical device, or browser. If an attempt fails, issue a new credential rather than retrying the old one. Do not expose tokens in screenshots, commits, or final responses. @@ -183,6 +181,8 @@ Keep local verification focused. Do not turn this workflow into a full repositor - **Old UI or an old error appears:** verify Metro's worktree, variant, URL, and port before diagnosing the app. - **The environment remains empty:** verify the platform-specific HTTP origin, use a fresh token, and confirm project seeding used the identical base directory. - **A second client cannot pair:** pairing tokens are single-use; issue another token. +- **The pairing form opens but does not connect:** confirm the deep link uses the existing `connections/new` route, includes `autoConnect=1`, and carries a freshly minted encoded `pairingUrl`. +- **Pairing text changes case or punctuation:** do not retry semantic typing. Use `scripts/pair-client.sh`; the simulator keyboard layout and HID input path are not reliable for credentials. - **iOS semantic actions fail:** set explicit XcodeBuildMCP defaults and refresh with `snapshot_ui`. - **Android cannot reach Metro:** verify `adb reverse` for the exact Metro port and relaunch the development-client URL. - **Android cannot reach the backend:** use `10.0.2.2`, not `127.0.0.1`, for the Android Emulator. diff --git a/.agents/skills/test-t3-mobile/scripts/pair-client.sh b/.agents/skills/test-t3-mobile/scripts/pair-client.sh new file mode 100755 index 000000000000..8023ba85f419 --- /dev/null +++ b/.agents/skills/test-t3-mobile/scripts/pair-client.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash + +set -euo pipefail + +usage() { + echo "Usage: $0 [url-scheme]" >&2 + exit 2 +} + +[[ $# -ge 4 && $# -le 5 ]] || usage + +platform="$1" +device_id="$2" +server_port="$3" +base_dir="$4" +url_scheme="${5:-t3code-dev}" + +case "$platform" in + ios) + mobile_origin="http://127.0.0.1:${server_port}" + ;; + android) + mobile_origin="http://10.0.2.2:${server_port}" + ;; + *) + usage + ;; +esac + +repo_root="$(git rev-parse --show-toplevel)" +cd "$repo_root" + +if ! pairing_output="$({ + MARCODE_PORT="$server_port" node apps/server/src/bin.ts auth pairing create \ + --base-dir "$base_dir" \ + --base-url "$mobile_origin" \ + --ttl 15m \ + --label "agent-mobile-${device_id:0:8}" +} 2>&1)"; then + echo "Could not mint a mobile pairing credential." >&2 + exit 1 +fi + +pairing_url="$(printf '%s\n' "$pairing_output" | sed -n 's/^Pair URL: //p' | tail -n 1)" +if [[ -z "$pairing_url" ]]; then + echo "Could not parse the mobile pairing URL." >&2 + exit 1 +fi + +deep_link="$(PAIRING_URL="$pairing_url" URL_SCHEME="$url_scheme" node - <<'NODE' +const query = new URLSearchParams({ + pairingUrl: process.env.PAIRING_URL, + autoConnect: "1", +}); +process.stdout.write(`${process.env.URL_SCHEME}://connections/new?${query}`); +NODE +)" + +case "$platform" in + ios) + xcrun simctl openurl "$device_id" "$deep_link" + ;; + android) + # adb shell re-joins its arguments and evaluates them through the device + # shell, so the deep link's `?`/`&` must be quoted once more for that shell. + adb -s "$device_id" shell \ + "am start -W -a android.intent.action.VIEW -d '$deep_link' com.t3tools.marcode.dev" \ + >/dev/null + ;; +esac + +echo "Opened the existing Add Environment route with a fresh pairing credential." diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index 9bc321dac0de..38a764eab6d7 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -9,6 +9,7 @@ body: attributes: value: | Use this form for broken behavior, regressions, crashes, or reliability problems. + Feature requests belong in [Discussions](https://github.com/pingdotgg/t3code/discussions/categories/ideas). Search existing issues first and keep the report focused on one problem. - type: checkboxes diff --git a/.github/ISSUE_TEMPLATE/config.yml b/.github/ISSUE_TEMPLATE/config.yml new file mode 100644 index 000000000000..4f4940ba6655 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/config.yml @@ -0,0 +1,5 @@ +blank_issues_enabled: false +contact_links: + - name: Feature request + url: https://github.com/pingdotgg/t3code/discussions/categories/ideas + about: Suggest an improvement or new capability in Discussions. diff --git a/.github/ISSUE_TEMPLATE/feature_request.yml b/.github/ISSUE_TEMPLATE/feature_request.yml deleted file mode 100644 index 3c9424fb322c..000000000000 --- a/.github/ISSUE_TEMPLATE/feature_request.yml +++ /dev/null @@ -1,102 +0,0 @@ -name: Feature request -description: Propose a scoped improvement or new capability. -title: "[Feature]: " -labels: - - enhancement - - needs-triage -body: - - type: markdown - attributes: - value: | - Use this form for new capabilities or meaningful improvements to existing behavior. - This repo is still early. Small, concrete requests that clearly explain the problem and scope are much easier to evaluate. - - - type: checkboxes - id: checks - attributes: - label: Before submitting - options: - - label: I searched existing issues and did not find a duplicate. - required: true - - label: I am describing a concrete problem or use case, not just a vague idea. - required: true - - - type: dropdown - id: area - attributes: - label: Area - description: Which part of the project would this change affect? - options: - - apps/web - - apps/server - - apps/desktop - - apps/mobile - - packages/contracts or packages/shared - - Build, CI, or release tooling - - Docs - - Not sure - validations: - required: true - - - type: textarea - id: problem - attributes: - label: Problem or use case - description: What are you trying to do? What is hard, slow, or impossible today? - placeholder: I want to reconnect to an existing provider session after a browser refresh without losing the current thread state. - validations: - required: true - - - type: textarea - id: proposal - attributes: - label: Proposed solution - description: Describe the behavior, API, or UX you want. - placeholder: Persist enough session metadata so the client can discover and reattach to the active provider session on load. - validations: - required: true - - - type: textarea - id: value - attributes: - label: Why this matters - description: Who benefits, and what outcome does this unlock? - placeholder: This would make reconnects predictable during network drops and reduce accidental duplicate sessions. - validations: - required: true - - - type: textarea - id: scope - attributes: - label: Smallest useful scope - description: What is the narrowest version of this request that would still solve your problem? - placeholder: A first pass only needs to support restoring the active session for the current thread. - validations: - required: true - - - type: textarea - id: alternatives - attributes: - label: Alternatives considered - description: Workarounds, prior art, or other approaches you considered. - placeholder: I currently work around this by manually restarting the provider session, but that loses in-flight context. - - - type: textarea - id: tradeoffs - attributes: - label: Risks or tradeoffs - description: What costs, complexity, or edge cases should be considered? - placeholder: This may require careful handling when the underlying provider session has already exited. - - - type: textarea - id: references - attributes: - label: Examples or references - description: Links, screenshots, mockups, or comparable tools. - - - type: checkboxes - id: contribution - attributes: - label: Contribution - options: - - label: I would be open to helping implement this. diff --git a/.github/VOUCHED.td b/.github/VOUCHED.td index 29910f522516..71e576e5c7e4 100644 --- a/.github/VOUCHED.td +++ b/.github/VOUCHED.td @@ -38,3 +38,4 @@ github:jappyjan github:justsomelegs github:UtkarshUsername github:SunkenInTime +github:bil0000 diff --git a/.github/pr-assets/6424-after.svg b/.github/pr-assets/6424-after.svg new file mode 100644 index 000000000000..dbeb594a09da --- /dev/null +++ b/.github/pr-assets/6424-after.svg @@ -0,0 +1 @@ + diff --git a/.github/pr-assets/6424-before.svg b/.github/pr-assets/6424-before.svg new file mode 100644 index 000000000000..6b365bad6e69 --- /dev/null +++ b/.github/pr-assets/6424-before.svg @@ -0,0 +1 @@ + diff --git a/.github/pr-assets/6503-after.svg b/.github/pr-assets/6503-after.svg new file mode 100644 index 000000000000..db1c9cb54065 --- /dev/null +++ b/.github/pr-assets/6503-after.svg @@ -0,0 +1 @@ + diff --git a/.github/workflows/mobile-showcase-screenshots.yml b/.github/workflows/mobile-showcase-screenshots.yml index 3eaaf508e31f..c64bccacdca8 100644 --- a/.github/workflows/mobile-showcase-screenshots.yml +++ b/.github/workflows/mobile-showcase-screenshots.yml @@ -21,6 +21,19 @@ on: - both - dark - light + theme: + description: Palette to capture (all multiplies the run by six) + required: true + default: t3-code + type: choice + options: + - t3-code + - t3-chat + - grove + - ocean + - ember + - iris + - all permissions: contents: read @@ -33,7 +46,9 @@ jobs: name: iPhone 6.9, iPhone 6.5, and iPad 13 if: inputs.platform == 'all' || inputs.platform == 'ios' runs-on: blacksmith-12vcpu-macos-26 - timeout-minutes: 60 + # Capturing every palette multiplies the device matrix by six, and only the + # one native build is shared between them. + timeout-minutes: ${{ inputs.theme == 'all' && 300 || 60 }} steps: - name: Checkout uses: actions/checkout@v6 @@ -62,10 +77,10 @@ jobs: "$vp_pnpm_bin/pnpm" --version - name: Capture iOS showcase - run: pnpm screenshots:mobile --platform ios --appearance "${{ inputs.appearance }}" + run: pnpm screenshots:mobile --platform ios --appearance "${{ inputs.appearance }}" --theme "${{ inputs.theme }}" - name: Validate App Store Connect assets - run: pnpm screenshots:mobile --platform ios --appearance "${{ inputs.appearance }}" --validate-only + run: pnpm screenshots:mobile --platform ios --appearance "${{ inputs.appearance }}" --theme "${{ inputs.theme }}" --validate-only - name: Upload iOS screenshots if: always() @@ -80,7 +95,9 @@ jobs: name: Android phone, 7-inch tablet, and 10-inch tablet if: inputs.platform == 'all' || inputs.platform == 'android' runs-on: blacksmith-16vcpu-ubuntu-2404 - timeout-minutes: 60 + # Capturing every palette multiplies the device matrix by six, and only the + # one native build is shared between them. + timeout-minutes: ${{ inputs.theme == 'all' && 300 || 60 }} env: T3_SHOWCASE_ANDROID_ABI: x86_64 steps: @@ -137,10 +154,10 @@ jobs: cores: 8 ram-size: 4096M disable-animations: false - script: pnpm screenshots:mobile --platform android --appearance "${{ inputs.appearance }}" + script: pnpm screenshots:mobile --platform android --appearance "${{ inputs.appearance }}" --theme "${{ inputs.theme }}" - name: Validate Google Play assets - run: pnpm screenshots:mobile --platform android --appearance "${{ inputs.appearance }}" --validate-only + run: pnpm screenshots:mobile --platform android --appearance "${{ inputs.appearance }}" --theme "${{ inputs.theme }}" --validate-only - name: Upload Android screenshots if: always() diff --git a/.github/workflows/publish-aur.yml b/.github/workflows/publish-aur.yml new file mode 100644 index 000000000000..62f8fd1f5470 --- /dev/null +++ b/.github/workflows/publish-aur.yml @@ -0,0 +1,65 @@ +name: Publish AUR package + +# See packaging/aur/README.md. + +on: + workflow_call: + inputs: + release_tag: + required: true + type: string + pkgrel: + required: false + default: "1" + type: string + secrets: + AUR_SSH_PRIVATE_KEY: + required: true + workflow_dispatch: + inputs: + release_tag: + description: "Release tag to publish" + required: true + type: string + pkgrel: + description: "Arch package release override" + required: false + default: "1" + type: string + +permissions: + contents: read + +concurrency: + group: publish-aur + cancel-in-progress: false + +jobs: + publish: + name: Validate and publish + runs-on: blacksmith-8vcpu-ubuntu-2404 + timeout-minutes: 30 + container: + image: archlinux:base-devel + + steps: + - name: Install Arch packaging tools + run: pacman -Syu --noconfirm --needed git github-cli jq namcap openssh sudo + + - name: Checkout packaging sources + uses: actions/checkout@v6 + + - name: Create unprivileged build user + run: | + useradd --create-home builder + install -Dm0440 /dev/stdin /etc/sudoers.d/builder <<'EOF' + builder ALL=(root) NOPASSWD: /usr/bin/pacman + EOF + + - name: Validate and publish package sources + env: + GH_TOKEN: ${{ github.token }} + RELEASE_TAG: ${{ inputs.release_tag }} + PKGREL: ${{ inputs.pkgrel || '1' }} + AUR_SSH_PRIVATE_KEY: ${{ secrets.AUR_SSH_PRIVATE_KEY }} + run: packaging/aur/scripts/release.sh diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 6d0150fb5740..6ebb6595777f 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -858,6 +858,16 @@ jobs: fail_on_unmatched_files: true token: ${{ github.token }} + publish_aur: + name: Publish AUR package + needs: [preflight, release] + if: ${{ !failure() && !cancelled() && needs.preflight.result == 'success' && needs.release.result == 'success' }} + uses: ./.github/workflows/publish-aur.yml + with: + release_tag: ${{ needs.preflight.outputs.tag }} + secrets: + AUR_SSH_PRIVATE_KEY: ${{ secrets.AUR_SSH_PRIVATE_KEY }} + deploy_web: name: Deploy hosted web app needs: [preflight, relay_public_config, release] diff --git a/.macroscope/check-run-agents/ui-consistency.md b/.macroscope/check-run-agents/ui-consistency.md new file mode 100644 index 000000000000..8ec720742759 --- /dev/null +++ b/.macroscope/check-run-agents/ui-consistency.md @@ -0,0 +1,82 @@ +--- +title: UI Consistency +model: claude-opus-5 +effort: high +input: full_diff +tools: + - browse_code + - git_tools + - github_api_read_only + - modify_pr +include: + - "apps/web/src/**/*.ts" + - "apps/web/src/**/*.tsx" + - "apps/web/src/**/*.css" +conclusion: failure +showToolCalls: true +--- + +# UI consistency review + +Review changed web UI code and directly affected call sites for consistency with the shared component system, Tailwind ownership, and the behavioral constraints below. Apply these rules when a pull request creates, moves, or modifies controls or styling. Do not demand unrelated repository-wide cleanup. + +The goal is not to minimize CSS or class counts at any cost. The goal is to put each behavior in the smallest correct owner while preserving interaction, theming, accessibility, layout, and browser behavior. + +## Shared controls and variants + +- Prefer the core UI primitives in `apps/web/src/components/ui` over native controls or locally reconstructed primitives. In ordinary product UI, a raw ` + ); diff --git a/apps/web/src/components/chat/TraitsPicker.tsx b/apps/web/src/components/chat/TraitsPicker.tsx index 8462757700e7..924fbddeab70 100644 --- a/apps/web/src/components/chat/TraitsPicker.tsx +++ b/apps/web/src/components/chat/TraitsPicker.tsx @@ -328,16 +328,23 @@ export const TraitsMenuContent = memo(function TraitsMenuContentImpl({ closeOnClick disabled={ultrathinkInBodyText && descriptor.id === primarySelectDescriptor?.id} > - - - {option.label} - {option.isDefault ? ( - <> - {" "} - - - ) : null} + + + + {option.label} + {option.isDefault ? ( + <> + {" "} + + + ) : null} + + {option.description ? ( + + {option.description} + + ) : null} ))} diff --git a/apps/web/src/components/chat/composerSubmission.test.ts b/apps/web/src/components/chat/composerSubmission.test.ts new file mode 100644 index 000000000000..239db28a6002 --- /dev/null +++ b/apps/web/src/components/chat/composerSubmission.test.ts @@ -0,0 +1,170 @@ +import { PROVIDER_SEND_TURN_MAX_INPUT_CHARS } from "@t3tools/contracts"; +import { describe, expect, it, vi } from "vite-plus/test"; + +import { submitComposerDraft } from "./composerSubmission"; + +describe("submitComposerDraft", () => { + it("keeps an oversized draft editable and sends a corrected follow-up", () => { + let draft = "x".repeat(PROVIDER_SEND_TURN_MAX_INPUT_CHARS + 1); + let validationMessage: string | null = null; + const dispatchedDrafts: string[] = []; + const preventDefault = vi.fn(); + + const submit = () => { + const result = submitComposerDraft({ + prompt: draft, + submissionTarget: "provider-turn", + event: { preventDefault }, + onSend: () => { + dispatchedDrafts.push(draft); + }, + }); + validationMessage = result.validationMessage; + }; + + submit(); + + expect(dispatchedDrafts).toEqual([]); + expect(draft).toHaveLength(PROVIDER_SEND_TURN_MAX_INPUT_CHARS + 1); + expect(validationMessage).toBe( + "Prompt is 1 character over the 120,000-character limit. Shorten or split it before sending.", + ); + expect(preventDefault).toHaveBeenCalledOnce(); + + draft = "Corrected prompt"; + submit(); + + expect(dispatchedDrafts).toEqual(["Corrected prompt"]); + expect(validationMessage).toBeNull(); + }); + + it("allows a draft at the shared character limit through the normal send path", () => { + const draft = "x".repeat(PROVIDER_SEND_TURN_MAX_INPUT_CHARS); + const onSend = vi.fn(); + const preventDefault = vi.fn(); + + const result = submitComposerDraft({ + prompt: draft, + submissionTarget: "provider-turn", + event: { preventDefault }, + onSend, + }); + + expect(result).toEqual({ validationMessage: null, didDispatch: true }); + expect(onSend).toHaveBeenCalledOnce(); + expect(preventDefault).not.toHaveBeenCalled(); + }); + + it("blocks when appended context pushes the provider input over the shared limit", () => { + const draft = "x".repeat(PROVIDER_SEND_TURN_MAX_INPUT_CHARS); + const onSend = vi.fn(); + + const result = submitComposerDraft({ + prompt: draft, + providerInput: `${draft}\n\nTerminal context`, + submissionTarget: "provider-turn", + event: undefined, + onSend, + }); + + expect(result).toEqual({ + validationMessage: + "Prompt is 18 characters over the 120,000-character limit. Shorten or split it before sending.", + didDispatch: false, + }); + expect(draft).toHaveLength(PROVIDER_SEND_TURN_MAX_INPUT_CHARS); + expect(onSend).not.toHaveBeenCalled(); + + const correctedResult = submitComposerDraft({ + prompt: "Corrected prompt", + providerInput: "Corrected prompt\n\nShort terminal context", + submissionTarget: "provider-turn", + event: undefined, + onSend, + }); + + expect(correctedResult).toEqual({ validationMessage: null, didDispatch: true }); + expect(onSend).toHaveBeenCalledOnce(); + }); + + it("does not finish submission when the send boundary rejects composed provider input", () => { + const preventDefault = vi.fn(); + + const result = submitComposerDraft({ + prompt: "Sendable raw draft", + submissionTarget: "provider-turn", + event: { preventDefault }, + onSend: () => false, + }); + + expect(result).toEqual({ validationMessage: null, didDispatch: false }); + expect(preventDefault).toHaveBeenCalledOnce(); + }); + + it("allows fully composed provider input at the shared character limit", () => { + const onSend = vi.fn(); + + const result = submitComposerDraft({ + prompt: "Short draft", + providerInput: "x".repeat(PROVIDER_SEND_TURN_MAX_INPUT_CHARS), + submissionTarget: "provider-turn", + event: undefined, + onSend, + }); + + expect(result).toEqual({ validationMessage: null, didDispatch: true }); + expect(onSend).toHaveBeenCalledOnce(); + }); + + it("blocks a generated plan follow-up that exceeds the shared limit", () => { + const onSend = vi.fn(); + + const result = submitComposerDraft({ + prompt: "", + providerInput: `PLEASE IMPLEMENT THIS PLAN:\n${"x".repeat( + PROVIDER_SEND_TURN_MAX_INPUT_CHARS, + )}`, + submissionTarget: "provider-turn", + event: undefined, + onSend, + }); + + expect(result.didDispatch).toBe(false); + expect(result.validationMessage).toContain("over the 120,000-character limit"); + expect(onSend).not.toHaveBeenCalled(); + }); + + it("allows surrounding whitespace that the provider turn contract trims", () => { + const draft = ` ${"x".repeat(PROVIDER_SEND_TURN_MAX_INPUT_CHARS)} `; + const onSend = vi.fn(); + const preventDefault = vi.fn(); + + const result = submitComposerDraft({ + prompt: draft, + submissionTarget: "provider-turn", + event: { preventDefault }, + onSend, + }); + + expect(result).toEqual({ validationMessage: null, didDispatch: true }); + expect(onSend).toHaveBeenCalledOnce(); + expect(preventDefault).not.toHaveBeenCalled(); + }); + + it("dispatches pending user input answers on their separate response path", () => { + const answer = "x".repeat(PROVIDER_SEND_TURN_MAX_INPUT_CHARS + 1); + const onSend = vi.fn(); + const preventDefault = vi.fn(); + + const result = submitComposerDraft({ + prompt: answer, + submissionTarget: "pending-user-input", + event: { preventDefault }, + onSend, + }); + + expect(result).toEqual({ validationMessage: null, didDispatch: true }); + expect(onSend).toHaveBeenCalledOnce(); + expect(preventDefault).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/components/chat/composerSubmission.ts b/apps/web/src/components/chat/composerSubmission.ts new file mode 100644 index 000000000000..528ac75bcabe --- /dev/null +++ b/apps/web/src/components/chat/composerSubmission.ts @@ -0,0 +1,44 @@ +import { PROVIDER_SEND_TURN_MAX_INPUT_CHARS } from "@t3tools/contracts"; + +type ComposerSubmitEvent = { preventDefault: () => void }; + +type ComposerSubmissionInput = { + prompt: string; + providerInput?: string; + submissionTarget: "provider-turn" | "pending-user-input"; +}; + +export function getComposerPromptLengthValidationMessage(prompt: string): string | null { + const excessCharacters = prompt.trim().length - PROVIDER_SEND_TURN_MAX_INPUT_CHARS; + if (excessCharacters <= 0) return null; + + const characterLabel = excessCharacters === 1 ? "character" : "characters"; + return `Prompt is ${excessCharacters.toLocaleString("en-US")} ${characterLabel} over the ${PROVIDER_SEND_TURN_MAX_INPUT_CHARS.toLocaleString("en-US")}-character limit. Shorten or split it before sending.`; +} + +export function getComposerSubmissionValidationMessage( + options: ComposerSubmissionInput, +): string | null { + return options.submissionTarget === "provider-turn" + ? getComposerPromptLengthValidationMessage(options.providerInput ?? options.prompt) + : null; +} + +export function submitComposerDraft( + options: ComposerSubmissionInput & { + event: ComposerSubmitEvent | undefined; + onSend: (event?: ComposerSubmitEvent) => boolean | void; + }, +): { validationMessage: string | null; didDispatch: boolean } { + const validationMessage = getComposerSubmissionValidationMessage(options); + if (validationMessage) { + options.event?.preventDefault(); + return { validationMessage, didDispatch: false }; + } + + if (options.onSend(options.event) === false) { + options.event?.preventDefault(); + return { validationMessage: null, didDispatch: false }; + } + return { validationMessage: null, didDispatch: true }; +} diff --git a/apps/web/src/components/chat/externalLinkContextMenu.test.ts b/apps/web/src/components/chat/externalLinkContextMenu.test.ts index 64935d53e46c..4f3dd1a153de 100644 --- a/apps/web/src/components/chat/externalLinkContextMenu.test.ts +++ b/apps/web/src/components/chat/externalLinkContextMenu.test.ts @@ -41,6 +41,25 @@ describe("external chat link context menu", () => { expect(harness.copyLink).not.toHaveBeenCalled(); }); + it("still offers the link's own actions where the integrated browser cannot be opened", async () => { + const harness = createHarness(null); + + await showExternalLinkContextMenu({ + href: "https://github.com/pingdotgg/t3code/pull/6169", + canOpenInPreview: false, + position: { x: 4, y: 8 }, + ...harness, + }); + + expect(harness.showContextMenu).toHaveBeenCalledWith( + [ + { id: "open-external", label: "Open in system browser" }, + { id: "copy-link", label: "Copy Link" }, + ], + { x: 4, y: 8 }, + ); + }); + it("copies the exact destination without opening it", async () => { const harness = createHarness("copy-link"); const href = "https://example.com/docs?topic=menus#copy"; diff --git a/apps/web/src/components/chat/externalLinkContextMenu.ts b/apps/web/src/components/chat/externalLinkContextMenu.ts index 398ca40da511..e93061c9fcb2 100644 --- a/apps/web/src/components/chat/externalLinkContextMenu.ts +++ b/apps/web/src/components/chat/externalLinkContextMenu.ts @@ -20,9 +20,25 @@ const EXTERNAL_LINK_CONTEXT_MENU_ITEMS = [ { id: "copy-link", label: "Copy Link" }, ] as const satisfies readonly ContextMenuItem[]; +/** + * The integrated browser is not always there to offer — it needs a thread to open beside and a + * runtime that can show it — but the other two answers hold wherever a link does. Dropping the + * whole menu with the one item that cannot be honoured is what left a right-click on a link + * showing the platform's cut-and-paste menu instead of a way to copy the link. + */ +export function externalLinkContextMenuItems(options: { + readonly canOpenInPreview: boolean; +}): readonly ContextMenuItem[] { + return options.canOpenInPreview + ? EXTERNAL_LINK_CONTEXT_MENU_ITEMS + : EXTERNAL_LINK_CONTEXT_MENU_ITEMS.filter((item) => item.id !== "open-in-preview"); +} + interface ShowExternalLinkContextMenuOptions { readonly href: string; readonly position: { readonly x: number; readonly y: number }; + /** Absent means yes, which is what every caller before the browser could be missing meant. */ + readonly canOpenInPreview?: boolean; readonly showContextMenu: ( items: readonly ContextMenuItem[], position: { readonly x: number; readonly y: number }, @@ -50,6 +66,7 @@ export function resolveExternalWebLinkHost(href: string | undefined): string | n export async function showExternalLinkContextMenu({ href, position, + canOpenInPreview = true, showContextMenu, openInPreview, openExternal, @@ -58,7 +75,7 @@ export async function showExternalLinkContextMenu({ }: ShowExternalLinkContextMenuOptions): Promise { let action: ExternalLinkContextMenuAction | null; try { - action = await showContextMenu(EXTERNAL_LINK_CONTEXT_MENU_ITEMS, position); + action = await showContextMenu(externalLinkContextMenuItems({ canOpenInPreview }), position); } catch (cause) { reportFailure("show-link-context-menu", cause); return; diff --git a/apps/web/src/components/chat/workspaceFileDrop.test.ts b/apps/web/src/components/chat/workspaceFileDrop.test.ts new file mode 100644 index 000000000000..ec5d074a3eb7 --- /dev/null +++ b/apps/web/src/components/chat/workspaceFileDrop.test.ts @@ -0,0 +1,78 @@ +import { describe, expect, it, vi } from "@effect/vitest"; +import { + makeWorkspaceFileDropHandlers, + type WorkspaceFileDragEvent, + type WorkspaceFileDropHost, +} from "./workspaceFileDrop"; + +function makeDragEvent(options?: { + types?: string[]; + files?: File[]; + movedWithinTarget?: boolean; +}) { + const preventDefault = vi.fn(); + const event = { + dataTransfer: { + types: options?.types ?? ["Files"], + files: options?.files ?? [], + dropEffect: "none", + }, + relatedTarget: options?.movedWithinTarget ? ({} as EventTarget) : null, + currentTarget: { + contains: () => options?.movedWithinTarget ?? false, + }, + preventDefault, + } satisfies WorkspaceFileDragEvent; + return { event, preventDefault }; +} + +function makeHost() { + const setDragActive = vi.fn(); + const addFiles = vi.fn(); + const host = { setDragActive, addFiles } satisfies WorkspaceFileDropHost; + return { host, setDragActive, addFiles }; +} + +describe("makeWorkspaceFileDropHandlers", () => { + it("activates the target for an external file drag", () => { + const { host, setDragActive } = makeHost(); + const { event, preventDefault } = makeDragEvent(); + + makeWorkspaceFileDropHandlers(host).onDragEnter(event); + + expect(preventDefault).toHaveBeenCalledOnce(); + expect(setDragActive).toHaveBeenCalledWith(true); + }); + + it("ignores non-file drags", () => { + const { host, setDragActive } = makeHost(); + const { event, preventDefault } = makeDragEvent({ types: ["text/plain"] }); + + makeWorkspaceFileDropHandlers(host).onDragOver(event); + + expect(preventDefault).not.toHaveBeenCalled(); + expect(setDragActive).not.toHaveBeenCalled(); + }); + + it("does not flicker when the drag moves between children", () => { + const { host, setDragActive } = makeHost(); + const { event } = makeDragEvent({ movedWithinTarget: true }); + + const handlers = makeWorkspaceFileDropHandlers(host); + handlers.onDragEnter(event); + handlers.onDragLeave(event); + + expect(setDragActive).not.toHaveBeenCalled(); + }); + + it("forwards dropped files and clears the active state", () => { + const file = new File(["contents"], "example.txt", { type: "text/plain" }); + const { host, setDragActive, addFiles } = makeHost(); + const { event } = makeDragEvent({ files: [file] }); + + makeWorkspaceFileDropHandlers(host).onDrop(event); + + expect(setDragActive).toHaveBeenCalledWith(false); + expect(addFiles).toHaveBeenCalledWith([file]); + }); +}); diff --git a/apps/web/src/components/chat/workspaceFileDrop.ts b/apps/web/src/components/chat/workspaceFileDrop.ts new file mode 100644 index 000000000000..132a8051e159 --- /dev/null +++ b/apps/web/src/components/chat/workspaceFileDrop.ts @@ -0,0 +1,54 @@ +export interface WorkspaceFileDragEvent { + readonly dataTransfer: { + readonly types: ReadonlyArray; + readonly files: Iterable; + dropEffect: string; + }; + readonly relatedTarget: EventTarget | null; + readonly currentTarget: { + contains(target: Node | null): boolean; + }; + preventDefault(): void; +} + +export interface WorkspaceFileDropHost { + setDragActive(active: boolean): void; + addFiles(files: File[]): void; +} + +function isFileDrag(event: WorkspaceFileDragEvent): boolean { + return event.dataTransfer.types.includes("Files"); +} + +function movedWithinDropTarget(event: WorkspaceFileDragEvent): boolean { + return event.relatedTarget !== null && event.currentTarget.contains(event.relatedTarget as Node); +} + +export function makeWorkspaceFileDropHandlers(host: WorkspaceFileDropHost) { + return { + onDragEnter(event: WorkspaceFileDragEvent) { + if (!isFileDrag(event)) return; + event.preventDefault(); + if (movedWithinDropTarget(event)) return; + host.setDragActive(true); + }, + onDragOver(event: WorkspaceFileDragEvent) { + if (!isFileDrag(event)) return; + event.preventDefault(); + event.dataTransfer.dropEffect = "copy"; + host.setDragActive(true); + }, + onDragLeave(event: WorkspaceFileDragEvent) { + if (!isFileDrag(event)) return; + event.preventDefault(); + if (movedWithinDropTarget(event)) return; + host.setDragActive(false); + }, + onDrop(event: WorkspaceFileDragEvent) { + if (!isFileDrag(event)) return; + event.preventDefault(); + host.setDragActive(false); + host.addFiles(Array.from(event.dataTransfer.files)); + }, + }; +} diff --git a/apps/web/src/components/clerk/ClerkUserProfilePage.tsx b/apps/web/src/components/clerk/ClerkUserProfilePage.tsx new file mode 100644 index 000000000000..00f20e53fbe1 --- /dev/null +++ b/apps/web/src/components/clerk/ClerkUserProfilePage.tsx @@ -0,0 +1,86 @@ +import { RefreshCwIcon } from "lucide-react"; +import type { ReactNode } from "react"; + +import { cn } from "../../lib/utils"; +import { Button } from "../ui/button"; + +export function ClerkUserProfilePage({ + action, + children, + className, + description, + title, +}: { + readonly action?: ReactNode; + readonly children: ReactNode; + readonly className?: string; + readonly description?: ReactNode; + readonly title: ReactNode; +}) { + return ( +
+
+
+

{title}

+ {description ? ( +

+ {description} +

+ ) : null} +
+ {action ?
{action}
: null} +
+ + {children} +
+ ); +} + +export function ClerkUserProfileRefreshButton({ + className, + disabled = false, + isPending, + onClick, +}: { + readonly className?: string; + readonly disabled?: boolean; + readonly isPending: boolean; + readonly onClick: () => void; +}) { + return ( + + ); +} + +export function ClerkUserProfileRow({ + children, + className, + icon, +}: { + readonly children: ReactNode; + readonly className?: string; + readonly icon: ReactNode; +}) { + return ( +
  • +
    + +
    {children}
    +
    +
    • + ); +} diff --git a/apps/web/src/components/clerk/MobileClientsUserProfilePage.tsx b/apps/web/src/components/clerk/MobileClientsUserProfilePage.tsx index 87555c413507..d261d18dfca4 100644 --- a/apps/web/src/components/clerk/MobileClientsUserProfilePage.tsx +++ b/apps/web/src/components/clerk/MobileClientsUserProfilePage.tsx @@ -1,8 +1,7 @@ import type { RelayClientDeviceRecord } from "@t3tools/contracts/relay"; -import { RefreshCwIcon, SmartphoneIcon } from "lucide-react"; +import { SmartphoneIcon } from "lucide-react"; import { useManagedRelayDevices } from "../../cloud/managedRelayState"; -import { cn } from "../../lib/utils"; import { Badge } from "../ui/badge"; import { Button } from "../ui/button"; import { Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle } from "../ui/empty"; @@ -12,6 +11,11 @@ import { mobileClientPlatformLabel, mobileClientUpdatedAtLabel, } from "./MobileClientsUserProfilePage.logic"; +import { + ClerkUserProfilePage, + ClerkUserProfileRefreshButton, + ClerkUserProfileRow, +} from "./ClerkUserProfilePage"; const MOBILE_CLIENT_SKELETON_ROWS = ["primary", "secondary"] as const; @@ -31,53 +35,47 @@ function MobileClientStatusBadge({ function MobileClientRow({ device }: { readonly device: RelayClientDeviceRecord }) { return ( -
  • -
    -
    - -
    -
    -
    -
    -

    {device.label}

    -

    {mobileClientPlatformLabel(device)}

    -
    -

    - {mobileClientUpdatedAtLabel(device.updatedAt)} -

    -
    -
    - - -
    -

    - {mobileClientNotificationDetail(device)} + }> +

    +
    +

    + {device.label} +

    +

    + {mobileClientPlatformLabel(device)}

    +

    + {mobileClientUpdatedAtLabel(device.updatedAt)} +

    -
    • +
    + + +
    +

    + {mobileClientNotificationDetail(device)} +

    + ); } function MobileClientsSkeleton() { return ( -
    +
    {MOBILE_CLIENT_SKELETON_ROWS.map((row) => ( -
    +
    - +
    - + -
    - - +
    + +
    @@ -89,13 +87,13 @@ function MobileClientsSkeleton() { function EmptyMobileClients() { return ( - - + + - No mobile clients - + No mobile clients + Sign in to Marcode on your iPhone to register it for push notifications and Live Activities. @@ -112,29 +110,20 @@ export function MobileClientsUserProfilePage() { const hasErrorWithoutData = devicesState.error !== null && devicesState.data === null; return ( -
    -
    -
    -

    Mobile clients

    -

    - Devices registered to receive Marcode Connect activity from your environments. -

    -
    - -
    - -
    + /> + } + > +
    {devicesState.error ? (
    @@ -152,7 +141,7 @@ export function MobileClientsUserProfilePage() { {isInitialLoad ? ( ) : hasErrorWithoutData ? null : devices.length > 0 ? ( -
      +
        {devices.map((device) => ( ))} @@ -161,6 +150,6 @@ export function MobileClientsUserProfilePage() { )}
    -
    + ); } diff --git a/apps/web/src/components/clerk/T3ConnectSidebarSignIn.tsx b/apps/web/src/components/clerk/T3ConnectSidebarSignIn.tsx index e9b1157f3f51..910d867908ac 100644 --- a/apps/web/src/components/clerk/T3ConnectSidebarSignIn.tsx +++ b/apps/web/src/components/clerk/T3ConnectSidebarSignIn.tsx @@ -1,9 +1,10 @@ import { UserButton, useAuth } from "@clerk/react"; -import { LogInIcon, SmartphoneIcon } from "lucide-react"; +import { LogInIcon, ServerIcon, SmartphoneIcon } from "lucide-react"; import { hasCloudPublicConfig } from "../../cloud/publicConfig"; import { SidebarMenu, SidebarMenuButton, SidebarMenuItem } from "../ui/sidebar"; import { MobileClientsUserProfilePage } from "./MobileClientsUserProfilePage"; +import { T3ConnectUserProfilePage } from "./T3ConnectUserProfilePage"; import { useT3ConnectAuthPrompt } from "./useT3ConnectAuthPrompt"; export function T3ConnectSidebarSignIn() { @@ -39,6 +40,13 @@ function ConfiguredT3ConnectSidebarAvatar() { > + } + url="t3-connect" + > + + ); } diff --git a/apps/web/src/components/clerk/T3ConnectUserProfilePage.test.tsx b/apps/web/src/components/clerk/T3ConnectUserProfilePage.test.tsx new file mode 100644 index 000000000000..377c9c945559 --- /dev/null +++ b/apps/web/src/components/clerk/T3ConnectUserProfilePage.test.tsx @@ -0,0 +1,63 @@ +import type { EnvironmentId } from "@t3tools/contracts"; +import type { RelayClientEnvironmentRecord } from "@t3tools/contracts/relay"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vite-plus/test"; + +import { T3ConnectEnvironmentRow } from "./T3ConnectUserProfilePage"; + +const environment: RelayClientEnvironmentRecord = { + environmentId: "environment-1" as EnvironmentId, + label: "Studio Mac", + endpoint: { + httpBaseUrl: "https://studio.example.com", + wsBaseUrl: "wss://studio.example.com", + providerKind: "cloudflare_tunnel", + }, + linkedAt: "2026-08-12T12:00:00.000Z", +}; + +function renderRow({ + confirmationOpen = false, + mutationPending = false, +}: { + readonly confirmationOpen?: boolean; + readonly mutationPending?: boolean; +} = {}) { + return renderToStaticMarkup( + , + ); +} + +describe("T3 Connect environment row", () => { + it("keeps deregistration confirmation inline and collapsed by default", () => { + const markup = renderRow(); + + expect(markup).toContain("Studio Mac"); + expect(markup).toContain("Deregister"); + expect(markup).not.toContain("Deregister server"); + expect(markup).not.toContain("Confirm deregistration of Studio Mac"); + }); + + it("expands Clerk-style confirmation content beneath the environment row", () => { + const markup = renderRow({ confirmationOpen: true }); + + expect(markup).toContain("Deregister server"); + expect(markup).toContain("“Studio Mac” will be removed from this account."); + expect(markup).toContain("Confirm deregistration of Studio Mac"); + expect(markup).toContain("Local connections on your devices are not changed."); + expect(markup).toContain("Cancel"); + }); + + it("locks the confirmation actions while deregistration is pending", () => { + const markup = renderRow({ confirmationOpen: true, mutationPending: true }); + + expect(markup).toContain("Deregistering…"); + expect(markup.match(/ disabled=""/g)).toHaveLength(3); + }); +}); diff --git a/apps/web/src/components/clerk/T3ConnectUserProfilePage.tsx b/apps/web/src/components/clerk/T3ConnectUserProfilePage.tsx new file mode 100644 index 000000000000..430925ebcde8 --- /dev/null +++ b/apps/web/src/components/clerk/T3ConnectUserProfilePage.tsx @@ -0,0 +1,261 @@ +import { findErrorTraceId } from "@t3tools/client-runtime/errors"; +import { + isAtomCommandInterrupted, + squashAtomCommandFailure, +} from "@t3tools/client-runtime/state/runtime"; +import type { EnvironmentId } from "@t3tools/contracts"; +import type { RelayClientEnvironmentRecord } from "@t3tools/contracts/relay"; +import { ServerIcon } from "lucide-react"; +import { useRef, useState } from "react"; + +import { + deregisterManagedRelayEnvironmentCommand, + useManagedRelayEnvironments, +} from "../../cloud/managedRelayState"; +import { useAtomCommand } from "../../state/use-atom-command"; +import { Button } from "../ui/button"; +import { Collapsible, CollapsiblePanel, CollapsibleTrigger } from "../ui/collapsible"; +import { Empty, EmptyDescription, EmptyHeader, EmptyMedia, EmptyTitle } from "../ui/empty"; +import { toastManager } from "../ui/toast"; +import { + ClerkUserProfilePage, + ClerkUserProfileRefreshButton, + ClerkUserProfileRow, +} from "./ClerkUserProfilePage"; + +const linkedAtFormatter = new Intl.DateTimeFormat(undefined, { dateStyle: "medium" }); + +function linkedAtLabel(value: string): string { + const linkedAt = new Date(value); + return Number.isNaN(linkedAt.getTime()) + ? "Link date unavailable" + : `Linked ${linkedAtFormatter.format(linkedAt)}`; +} + +function endpointLabel(environment: RelayClientEnvironmentRecord): string { + return environment.endpoint.providerKind === "cloudflare_tunnel" + ? "Managed tunnel" + : "Activity publishing only"; +} + +export function T3ConnectEnvironmentRow(props: { + readonly environment: RelayClientEnvironmentRecord; + readonly confirmationOpen: boolean; + readonly mutationPending: boolean; + readonly onConfirmationChange: (open: boolean) => void; + readonly onDeregister: (environment: RelayClientEnvironmentRecord) => void; +}) { + const { environment } = props; + return ( + }> + +
    +
    +

    + {environment.label} +

    +

    + {linkedAtLabel(environment.linkedAt)} · {endpointLabel(environment)} +

    +
    + + Deregister + + } + /> +
    + + +
    +
    +

    + Deregister server +

    +

    + “{environment.label}” will be removed from this account. +

    +

    + Marcode Connect access will be revoked, any managed tunnel will be removed, and a + host space will become available. Local connections on your devices are not changed. +

    +
    + + +
    +
    +
    +     +     +     + ); +} + +export function T3ConnectUserProfilePage() { + const environmentsState = useManagedRelayEnvironments(); + const deregisterEnvironment = useAtomCommand(deregisterManagedRelayEnvironmentCommand, { + reportFailure: false, + }); + const [deregisteringEnvironmentId, setDeregisteringEnvironmentId] = + useState(null); + const [confirmingEnvironmentId, setConfirmingEnvironmentId] = useState( + null, + ); + const mutationPendingRef = useRef(false); + const [removedEnvironments, setRemovedEnvironments] = useState<{ + readonly accountId: string | null; + readonly linkedAtById: ReadonlyMap; + }>({ accountId: null, linkedAtById: new Map() }); + + const handleDeregister = async (environment: RelayClientEnvironmentRecord) => { + const accountId = environmentsState.accountId; + if (!accountId || mutationPendingRef.current) return; + + mutationPendingRef.current = true; + setDeregisteringEnvironmentId(environment.environmentId); + const result = await deregisterEnvironment({ + accountId, + environmentId: environment.environmentId, + }); + mutationPendingRef.current = false; + setDeregisteringEnvironmentId(null); + + if (result._tag === "Success") { + setConfirmingEnvironmentId(null); + setRemovedEnvironments((current) => { + const linkedAtById = new Map(current.accountId === accountId ? current.linkedAtById : []); + linkedAtById.set(environment.environmentId, environment.linkedAt); + return { accountId, linkedAtById }; + }); + environmentsState.refresh(); + toastManager.add({ + type: "success", + title: "Server deregistered", + description: "Marcode Connect access was revoked and a host space is now available.", + }); + return; + } + if (isAtomCommandInterrupted(result)) return; + + const cause = squashAtomCommandFailure(result); + const message = cause instanceof Error ? cause.message : "Could not deregister the server."; + const traceId = findErrorTraceId(cause); + console.error("[t3-connect] Could not deregister environment", { + environmentId: environment.environmentId, + message, + traceId, + cause, + }); + toastManager.add({ + type: "error", + title: "Could not deregister server", + description: message, + data: traceId + ? { + secondaryActionProps: { + children: "Copy trace ID", + onClick: () => void navigator.clipboard?.writeText(traceId), + }, + } + : undefined, + }); + }; + + const removedEnvironmentLinkedAt = + removedEnvironments.accountId === environmentsState.accountId + ? removedEnvironments.linkedAtById + : new Map(); + const environments = (environmentsState.data ?? []).filter( + (environment) => + removedEnvironmentLinkedAt.get(environment.environmentId) !== environment.linkedAt, + ); + const isInitialLoad = + !environmentsState.accountId || (environmentsState.data === null && !environmentsState.error); + + return ( + + } + > +
    + {environmentsState.error ? ( +
    +

    + Could not load Marcode Connect environments +

    +

    {environmentsState.error}

    +
    + ) : null} + + {isInitialLoad ? ( +

    + Loading environments… +

    + ) : environments.length > 0 ? ( +
      + {environments.map((environment) => ( + + setConfirmingEnvironmentId(open ? environment.environmentId : null) + } + onDeregister={(selected) => void handleDeregister(selected)} + /> + ))} +
    + ) : environmentsState.error ? null : ( + + + + + + + No Marcode Connect environments + + + Link an environment from its local Settings to make it available through Marcode + Connect. + + + + )} +
    +     + ); +} diff --git a/apps/web/src/components/clerk/authRedirect.test.ts b/apps/web/src/components/clerk/authRedirect.test.ts index 140474120cca..e948d1d9c049 100644 --- a/apps/web/src/components/clerk/authRedirect.test.ts +++ b/apps/web/src/components/clerk/authRedirect.test.ts @@ -5,7 +5,10 @@ import { resolveClerkSignInProps } from "./authRedirect"; describe("resolveClerkSignInProps", () => { it("returns to the current browser URL on the web", () => { const href = "https://app.t3.codes/connect?state=state-1#details"; - expect(resolveClerkSignInProps(href, false)).toEqual({ forceRedirectUrl: href }); + expect(resolveClerkSignInProps(href, false)).toEqual({ + forceRedirectUrl: href, + signUpForceRedirectUrl: href, + }); }); it("removes a Clerk virtual pathname and callback params while preserving the desktop route", () => { diff --git a/apps/web/src/components/clerk/authRedirect.ts b/apps/web/src/components/clerk/authRedirect.ts index 251c5ee36502..e0b07241c068 100644 --- a/apps/web/src/components/clerk/authRedirect.ts +++ b/apps/web/src/components/clerk/authRedirect.ts @@ -15,5 +15,7 @@ export function resolveClerkSignInProps(href: string, isElectron: boolean): Cler signUpForceRedirectUrl: redirectUrl.toString(), }; } - return { forceRedirectUrl: href }; + // The sign-in modal can switch to sign-up, which follows its own redirect + // target; without one Clerk falls back to the URL the modal was opened from. + return { forceRedirectUrl: href, signUpForceRedirectUrl: href }; } diff --git a/apps/web/src/components/cloud/ConnectCliAuthSurface.tsx b/apps/web/src/components/cloud/ConnectCliAuthSurface.tsx index 9e8cfa815103..b19a37f6015d 100644 --- a/apps/web/src/components/cloud/ConnectCliAuthSurface.tsx +++ b/apps/web/src/components/cloud/ConnectCliAuthSurface.tsx @@ -1,9 +1,10 @@ import { useAuth, useClerk, useUser } from "@clerk/react"; import { encodeConnectAuthCode, readConnectAuthorizeRequest } from "@t3tools/shared/connectAuth"; -import { useEffect, useRef, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; import { buildConnectCliClerkAuthorizeUrl, + connectCliSignInRedirectUrl, readConnectCliAuthState, readConnectCliCallbackResult, rememberConnectCliAuthState, @@ -56,6 +57,21 @@ export function ConnectCliAuthorizeSurface() { const signInOpened = useRef(false); const redirecting = useRef(false); + const openSignIn = useCallback(() => { + if (!request) { + return; + } + // Clerk redirects to the authorize endpoint itself once sign-in completes, + // so the callback's state check has to be armed before handing off. + rememberConnectCliAuthState(request.state); + clerk.openSignIn( + resolveClerkSignInProps( + connectCliSignInRedirectUrl(request, window.location.href), + isElectron, + ), + ); + }, [clerk, request]); + useEffect(() => { if (!request || !isLoaded || redirecting.current) { return; @@ -63,7 +79,7 @@ export function ConnectCliAuthorizeSurface() { if (!isSignedIn) { if (!signInOpened.current) { signInOpened.current = true; - clerk.openSignIn(resolveClerkSignInProps(window.location.href, isElectron)); + openSignIn(); } return; } @@ -74,7 +90,7 @@ export function ConnectCliAuthorizeSurface() { redirecting.current = true; rememberConnectCliAuthState(request.state); window.location.assign(authorizeUrl); - }, [clerk, isLoaded, isSignedIn, request]); + }, [isLoaded, isSignedIn, openSignIn, request]); if (!request) { return ( @@ -101,12 +117,7 @@ export function ConnectCliAuthorizeSurface() { /> {isLoaded && !isSignedIn ? (
    -
    diff --git a/apps/web/src/components/composerFooterLayout.test.ts b/apps/web/src/components/composerFooterLayout.test.ts index b9f2a6b6a244..92e054df52dc 100644 --- a/apps/web/src/components/composerFooterLayout.test.ts +++ b/apps/web/src/components/composerFooterLayout.test.ts @@ -3,7 +3,6 @@ import { describe, expect, it } from "vite-plus/test"; import { COMPOSER_FOOTER_COMPACT_BREAKPOINT_PX, COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX, - COMPOSER_PRIMARY_ACTIONS_COMPACT_BREAKPOINT_PX, shouldUseCompactComposerPrimaryActions, shouldUseCompactComposerFooter, } from "./composerFooterLayout"; @@ -38,16 +37,14 @@ describe("shouldUseCompactComposerFooter", () => { describe("shouldUseCompactComposerPrimaryActions", () => { it("matches the wide footer breakpoint", () => { - expect(COMPOSER_PRIMARY_ACTIONS_COMPACT_BREAKPOINT_PX).toBe( - COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX, - ); expect( - shouldUseCompactComposerPrimaryActions(COMPOSER_PRIMARY_ACTIONS_COMPACT_BREAKPOINT_PX - 1, { - hasWideActions: true, - }), + shouldUseCompactComposerPrimaryActions( + COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX - 1, + { hasWideActions: true }, + ), ).toBe(true); expect( - shouldUseCompactComposerPrimaryActions(COMPOSER_PRIMARY_ACTIONS_COMPACT_BREAKPOINT_PX, { + shouldUseCompactComposerPrimaryActions(COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX, { hasWideActions: true, }), ).toBe(false); diff --git a/apps/web/src/components/composerFooterLayout.ts b/apps/web/src/components/composerFooterLayout.ts index ae5fd56669f4..5e0b3a8ea379 100644 --- a/apps/web/src/components/composerFooterLayout.ts +++ b/apps/web/src/components/composerFooterLayout.ts @@ -1,7 +1,5 @@ export const COMPOSER_FOOTER_COMPACT_BREAKPOINT_PX = 620; export const COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX = 780; -export const COMPOSER_PRIMARY_ACTIONS_COMPACT_BREAKPOINT_PX = - COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX; export function shouldUseCompactComposerFooter( width: number | null, @@ -20,5 +18,5 @@ export function shouldUseCompactComposerPrimaryActions( if (!options?.hasWideActions) { return false; } - return width !== null && width < COMPOSER_PRIMARY_ACTIONS_COMPACT_BREAKPOINT_PX; + return width !== null && width < COMPOSER_FOOTER_WIDE_ACTIONS_COMPACT_BREAKPOINT_PX; } diff --git a/apps/web/src/components/composerInlineChip.ts b/apps/web/src/components/composerInlineChip.ts index c17b3ddab3c0..3f0e8ca1ac00 100644 --- a/apps/web/src/components/composerInlineChip.ts +++ b/apps/web/src/components/composerInlineChip.ts @@ -8,6 +8,9 @@ export const CHAT_INLINE_CHIP_CLASS_NAME = `${INLINE_CHIP_CLASS_NAME} text-[12px export const COMPOSER_INLINE_CHIP_CLASS_NAME = `${INLINE_CHIP_CLASS_NAME} text-[0.86em] select-none`; +export const COMPOSER_INLINE_CHIP_DECORATOR_CLASS_NAME = + "relative inline-flex align-[-0.125em] leading-none data-[composer-chip-selected]:after:pointer-events-none data-[composer-chip-selected]:after:absolute data-[composer-chip-selected]:after:inset-0 data-[composer-chip-selected]:after:rounded-[6px] data-[composer-chip-selected]:after:bg-[Highlight] data-[composer-chip-selected]:after:opacity-30 data-[composer-chip-selected]:after:content-['']"; + export const COMPOSER_INLINE_CHIP_ICON_CLASS_NAME = "size-[1.17em] shrink-0 opacity-85"; export const CHAT_INLINE_CHIP_LABEL_CLASS_NAME = "truncate leading-tight"; diff --git a/apps/web/src/components/desktopUpdate.logic.test.ts b/apps/web/src/components/desktopUpdate.logic.test.ts index 7e31fdfd9628..df7567d693ad 100644 --- a/apps/web/src/components/desktopUpdate.logic.test.ts +++ b/apps/web/src/components/desktopUpdate.logic.test.ts @@ -244,30 +244,15 @@ describe("desktop update UI helpers", () => { ).toContain("Install update and restart Marcode?"); }); - it("warns Windows users that a silent installation can take several minutes", () => { - const message = getDesktopUpdateInstallConfirmationMessage( - { - availableVersion: "1.1.0", - downloadedVersion: "1.1.0", - }, - "Win32", - ); - - expect(message).toContain("may remain closed for several minutes"); - expect(message).toContain("no installer window may appear"); - expect(message).toContain("will reopen automatically"); - }); - - it("keeps the additional silent installation warning Windows-specific", () => { - const message = getDesktopUpdateInstallConfirmationMessage( - { + it("keeps the same install confirmation copy across desktop platforms", () => { + expect( + getDesktopUpdateInstallConfirmationMessage({ availableVersion: "1.1.0", downloadedVersion: "1.1.0", - }, - "MacIntel", + }), + ).toBe( + "Install update 1.1.0 and restart Marcode?\n\nAny running tasks will be interrupted. Make sure you're ready before continuing.", ); - - expect(message).not.toContain("may remain closed for several minutes"); }); }); diff --git a/apps/web/src/components/desktopUpdate.logic.ts b/apps/web/src/components/desktopUpdate.logic.ts index 1dba5a471020..c560b37b3948 100644 --- a/apps/web/src/components/desktopUpdate.logic.ts +++ b/apps/web/src/components/desktopUpdate.logic.ts @@ -1,5 +1,4 @@ import type { DesktopUpdateActionResult, DesktopUpdateState } from "@t3tools/contracts"; -import { isWindowsPlatform } from "../lib/utils"; export type DesktopUpdateButtonAction = "download" | "install" | "none"; @@ -97,13 +96,9 @@ export function getDesktopUpdateButtonTooltip(state: DesktopUpdateState): string export function getDesktopUpdateInstallConfirmationMessage( state: Pick, - platform = "", ): string { const version = state.downloadedVersion ?? state.availableVersion; - const windowsInstallWarning = isWindowsPlatform(platform) - ? "\n\nOn Windows, Marcode may remain closed for several minutes while the update installs, and no installer window may appear. Marcode will reopen automatically when installation finishes." - : ""; - return `Install update${version ? ` ${version}` : ""} and restart Marcode?\n\nAny running tasks will be interrupted. Make sure you're ready before continuing.${windowsInstallWarning}`; + return `Install update${version ? ` ${version}` : ""} and restart Marcode?\n\nAny running tasks will be interrupted. Make sure you're ready before continuing.`; } export function getDesktopUpdateActionError(result: DesktopUpdateActionResult): string | null { diff --git a/apps/web/src/components/desktopUpdate.toast.tsx b/apps/web/src/components/desktopUpdate.toast.tsx index 004a76a81cd7..4e55f3a28d12 100644 --- a/apps/web/src/components/desktopUpdate.toast.tsx +++ b/apps/web/src/components/desktopUpdate.toast.tsx @@ -18,7 +18,7 @@ function ReleaseNotesLink({ }) { return ( ); } diff --git a/apps/web/src/components/diffs/DiffCommentAnnotation.test.tsx b/apps/web/src/components/diffs/DiffCommentAnnotation.test.tsx index 2c53c9059dcf..d430e3837148 100644 --- a/apps/web/src/components/diffs/DiffCommentAnnotation.test.tsx +++ b/apps/web/src/components/diffs/DiffCommentAnnotation.test.tsx @@ -40,21 +40,16 @@ describe("DiffCommentAnnotation", () => { {...callbacks} submitLabel="Add to review" secondaryAction={{ - label: "Ask", - icon: , - allowEmpty: true, + label: "Add to agent", onAction: vi.fn(), }} />, ); expect(markup).toContain("Add a comment…"); - expect(markup).toContain(">Ask"); expect(markup).toContain(">Add to review"); expect(markup.match(/]*disabled[^>]*>Add to review<\/button>/)).not.toBeNull(); - const askButton = markup.match(/]*>.*?Ask<\/button>/)?.[0]; - expect(askButton).toBeDefined(); - expect(askButton).not.toContain(' disabled=""'); + expect(markup.match(/]*disabled[^>]*>Add to agent<\/button>/)).not.toBeNull(); }); it("renders a saved comment without a nested card or redundant range label", () => { diff --git a/apps/web/src/components/diffs/StyledDiffCodeView.test.tsx b/apps/web/src/components/diffs/StyledDiffCodeView.test.tsx index 6f289de2bcbc..f0cd49abc41d 100644 --- a/apps/web/src/components/diffs/StyledDiffCodeView.test.tsx +++ b/apps/web/src/components/diffs/StyledDiffCodeView.test.tsx @@ -35,7 +35,9 @@ describe("StyledDiffCodeView", () => { />, ); - expect(testState.codeViewClassName).toBe("diff-render-surface outline-none min-h-0"); + expect(testState.codeViewClassName).toBe( + "diff-render-surface [--code-background:var(--background)] outline-none min-h-0", + ); expect(testState.codeViewOptions).toMatchObject({ theme: "pierre-dark", stickyHeaders: true, @@ -44,7 +46,7 @@ describe("StyledDiffCodeView", () => { diffHeaderHeight: 32, hunkSeparatorHeight: 24, paddingTop: 0, - paddingBottom: 0, + paddingBottom: 8, }, layout: { paddingTop: 0, paddingBottom: 0, gap: 0 }, }); diff --git a/apps/web/src/components/diffs/StyledDiffCodeView.tsx b/apps/web/src/components/diffs/StyledDiffCodeView.tsx index f422c7aebbd8..14939de09820 100644 --- a/apps/web/src/components/diffs/StyledDiffCodeView.tsx +++ b/apps/web/src/components/diffs/StyledDiffCodeView.tsx @@ -269,6 +269,11 @@ type StyledDiffCodeViewProps = ( ) & { readonly options?: StyledDiffCodeViewOptions; readonly viewerRef?: Ref>; + /** + * Appended to the shared stylesheet inside the viewer's shadow root, for a surface that has + * to restyle chrome the viewer owns — such as replacing its per-file line counts. + */ + readonly unsafeCSSExtra?: string; }; /** The shared web CodeView surface: app styling and virtualized geometry stay paired here. */ @@ -276,6 +281,7 @@ export function StyledDiffCodeView({ options, viewerRef, className, + unsafeCSSExtra, ...props }: StyledDiffCodeViewProps) { return ( @@ -286,21 +292,28 @@ export function StyledDiffCodeView({ // outside the panel clipping boundary; actual controls inside retain their own indicators. className={ className - ? `diff-render-surface outline-none ${className}` - : "diff-render-surface outline-none" + ? `diff-render-surface [--code-background:var(--background)] outline-none ${className}` + : "diff-render-surface [--code-background:var(--background)] outline-none" } options={{ ...options, - unsafeCSS: DIFF_VIEW_UNSAFE_CSS, + unsafeCSS: unsafeCSSExtra + ? `${DIFF_VIEW_UNSAFE_CSS}\n${unsafeCSSExtra}` + : DIFF_VIEW_UNSAFE_CSS, itemMetrics: { diffHeaderHeight: 32, hunkSeparatorHeight: 24, // Pierre uses its general file spacing as a fallback in expanded-file layout paths. - // Keep it zero alongside the explicit paddings or expanding the first file can + // Keep it zero alongside the explicit paddingTop or expanding the first file can // reintroduce the library's default 8px gap above its header. spacing: 0, paddingTop: 0, - paddingBottom: 0, + // Unlike the gap above, the 8px under a file's last line is painted + // unconditionally by Pierre's stylesheet (`--diffs-gap-fallback`), so the metric has + // to count it: at zero every expanded file's virtual height ran 8px short of its + // rendered height, and the end of the list sat past the reachable scroll range — + // one clipped file row per expanded file above it. + paddingBottom: 8, }, layout: { paddingTop: 0, paddingBottom: 0, gap: 0 }, }} diff --git a/apps/web/src/components/preview/PreviewAutomationHosts.tsx b/apps/web/src/components/preview/PreviewAutomationHosts.tsx index 2a007fb4ce57..acf7e52e3039 100644 --- a/apps/web/src/components/preview/PreviewAutomationHosts.tsx +++ b/apps/web/src/components/preview/PreviewAutomationHosts.tsx @@ -38,6 +38,7 @@ import { } from "~/browser/browserRecording"; import { resolveBrowserRecordingStopTarget } from "~/browser/browserRecordingScope"; import { useBrowserSurfaceStore } from "~/browser/browserSurfaceStore"; +import { browserDefaultOpenViewport, resolveBrowserDefaults } from "~/browser/browserDefaults"; import { runBrowserViewportMutation } from "~/browser/browserViewportActions"; import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId"; import { isElectron } from "~/env"; @@ -380,6 +381,9 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) input: { threadId: request.threadId, ...(resolvedInputUrl ? { url: resolvedInputUrl } : {}), + // An agent that didn't state a size gets the user's + // configured default, same as a hand-opened tab. + viewport: browserDefaultOpenViewport(await resolveBrowserDefaults()), }, }); if (result._tag === "Failure") { @@ -428,7 +432,10 @@ function PreviewAutomationHost(props: { readonly environmentId: EnvironmentId }) updatePreviewServerSnapshot(threadRef, resizeResult.value); } } - const shouldPresentPreview = shouldOpenPreviewMiniPlayer(input); + const shouldPresentPreview = shouldOpenPreviewMiniPlayer( + input, + (await resolveBrowserDefaults()).autoShowFloatingPreview, + ); if (shouldPresentPreview) { usePreviewMiniPlayerStore.getState().open(threadRef, activeTabId); } diff --git a/apps/web/src/components/preview/PreviewChromeRow.tsx b/apps/web/src/components/preview/PreviewChromeRow.tsx index 958b30a47978..49f0b9fa16f1 100644 --- a/apps/web/src/components/preview/PreviewChromeRow.tsx +++ b/apps/web/src/components/preview/PreviewChromeRow.tsx @@ -109,7 +109,11 @@ export function PreviewChromeRow({ return (
    -
    +
    - + ({ pid: number | null; terminal: null; source: "scanner"; - listening: boolean; }>, })); vi.mock("./useDiscoveredLocalServers", () => ({ useDiscoveredLocalServers: () => mocks.servers, })); +vi.mock("./PreviewFaviconIcon", () => ({ + PreviewFaviconIcon: () => , +})); import { PreviewEmptyState } from "./PreviewEmptyState"; const environmentId = EnvironmentId.make("env-1"); +const threadRef = { environmentId, threadId: ThreadId.make("thread-1") }; function server(port: number) { return { @@ -34,13 +37,13 @@ function server(port: number) { pid: 1, terminal: null, source: "scanner" as const, - listening: true, }; } function render(recentEntries: Array<{ url: string; lastVisitedAt: number; title?: string }>) { return renderToStaticMarkup( undefined} diff --git a/apps/web/src/components/preview/PreviewEmptyState.tsx b/apps/web/src/components/preview/PreviewEmptyState.tsx index 3b9aacf4dfd6..163849154000 100644 --- a/apps/web/src/components/preview/PreviewEmptyState.tsx +++ b/apps/web/src/components/preview/PreviewEmptyState.tsx @@ -1,4 +1,4 @@ -import type { EnvironmentId } from "@t3tools/contracts"; +import type { EnvironmentId, ScopedThreadRef } from "@t3tools/contracts"; import { Globe, History, RadioTower } from "lucide-react"; import type { BrowserHistoryEntry } from "~/browserHistoryStore"; @@ -9,18 +9,18 @@ import { PreviewRecentUrlCard } from "./PreviewRecentUrlCard"; import { useDiscoveredLocalServers } from "./useDiscoveredLocalServers"; interface Props { + threadRef: ScopedThreadRef; environmentId: EnvironmentId; configuredUrls?: ReadonlyArray | undefined; - recentlySeenUrls?: ReadonlyArray | undefined; recentEntries: ReadonlyArray; onRemoveRecent: (url: string) => void; onOpenUrl: (url: string) => void; } export function PreviewEmptyState({ + threadRef, environmentId, configuredUrls, - recentlySeenUrls, recentEntries, onRemoveRecent, onOpenUrl, @@ -28,7 +28,6 @@ export function PreviewEmptyState({ const servers = useDiscoveredLocalServers({ environmentId, configuredUrls, - recentlySeenUrls, }); const recents = recentEntries.filter((entry) => URL.canParse(entry.url)).slice(0, 8); @@ -40,7 +39,7 @@ export function PreviewEmptyState({ No preview yet - Type a URL above, or run a dev script. Listening localhost ports will show up here + Type a URL above, or run a dev script. Browser-ready localhost servers will show up here automatically. @@ -49,7 +48,7 @@ export function PreviewEmptyState({ return (
    -
    +
    {recents.length > 0 ? (
    @@ -60,6 +59,7 @@ export function PreviewEmptyState({ {recents.map((entry) => ( onOpenUrl(entry.url)} onRemove={() => onRemoveRecent(entry.url)} @@ -78,13 +78,14 @@ export function PreviewEmptyState({ {servers.map((server) => ( onOpenUrl(server.requestedUrl)} /> ))}

    - Select a listening port to open it in this browser tab. + Select a live local server to open it in this browser tab.

    ) : null} diff --git a/apps/web/src/components/preview/PreviewFaviconIcon.test.tsx b/apps/web/src/components/preview/PreviewFaviconIcon.test.tsx new file mode 100644 index 000000000000..d950a99b59fc --- /dev/null +++ b/apps/web/src/components/preview/PreviewFaviconIcon.test.tsx @@ -0,0 +1,51 @@ +import { EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { renderToStaticMarkup } from "react-dom/server"; +import { describe, expect, it, vi } from "vite-plus/test"; + +const mocks = vi.hoisted(() => ({ favicon: null as string | null })); + +vi.mock("~/browserFaviconStore", () => ({ + useFaviconForThreadUrl: () => mocks.favicon, +})); + +import { FaviconImage, PreviewFaviconIcon, selectFaviconSource } from "./PreviewFaviconIcon"; + +const threadRef = { + environmentId: EnvironmentId.make("env-1"), + threadId: ThreadId.make("thread-1"), +}; + +describe("preview favicon image", () => { + it("renders a captured source before later fallback sources", () => { + expect( + renderToStaticMarkup( + fallback} + />, + ), + ).toContain('src="data:image/png;base64,AAAA"'); + const captured = "data:image/png;base64,AAAA"; + const google = "https://public.example/icon"; + expect(selectFaviconSource([captured, google], new Set())).toBe(captured); + expect(selectFaviconSource([captured, google], new Set([captured]))).toBe(google); + expect(selectFaviconSource([captured, google], new Set([captured, google]))).toBeNull(); + expect(selectFaviconSource(["data:image/png;base64,BBBB", google], new Set([captured]))).toBe( + "data:image/png;base64,BBBB", + ); + }); + + it("uses a stored project icon or falls back to the browser mockup", () => { + mocks.favicon = null; + const html = renderToStaticMarkup( + , + ); + expect(html).not.toContain(", + ); + expect(faviconHtml).toContain('src="data:image/png;base64,AAAA"'); + }); +}); diff --git a/apps/web/src/components/preview/PreviewFaviconIcon.tsx b/apps/web/src/components/preview/PreviewFaviconIcon.tsx new file mode 100644 index 000000000000..111facfd82dd --- /dev/null +++ b/apps/web/src/components/preview/PreviewFaviconIcon.tsx @@ -0,0 +1,66 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; +import { type ReactNode, useState } from "react"; + +import { useFaviconForThreadUrl } from "~/browserFaviconStore"; +import { cn } from "~/lib/utils"; + +import { BrowserMockup } from "./BrowserMockup"; + +export function selectFaviconSource( + sources: ReadonlyArray, + failed: ReadonlySet, +): string | null { + return sources.find((candidate) => !failed.has(candidate)) ?? null; +} + +export function FaviconImage(props: { + sources: ReadonlyArray; + fallback: ReactNode; + className?: string | undefined; +}) { + const sources = props.sources.filter((source): source is string => Boolean(source)); + return ( + + ); +} + +function FaviconImageAttempt(props: { + sources: ReadonlyArray; + fallback: ReactNode; + className?: string | undefined; +}) { + const [failed, setFailed] = useState>(() => new Set()); + const source = selectFaviconSource(props.sources, failed); + if (!source) return props.fallback; + return ( + setFailed((current) => new Set(current).add(source))} + /> + ); +} + +export function PreviewFaviconIcon(props: { + threadRef: ScopedThreadRef; + url: string; + className?: string | undefined; +}) { + const source = useFaviconForThreadUrl(props.threadRef, props.url); + const fallback = ; + return ( + + ); +} diff --git a/apps/web/src/components/preview/PreviewLocalServerCard.tsx b/apps/web/src/components/preview/PreviewLocalServerCard.tsx index c7b08ad2893d..263cdb294f48 100644 --- a/apps/web/src/components/preview/PreviewLocalServerCard.tsx +++ b/apps/web/src/components/preview/PreviewLocalServerCard.tsx @@ -1,12 +1,15 @@ -import { BrowserMockup } from "./BrowserMockup"; +import type { ScopedThreadRef } from "@t3tools/contracts"; + +import { PreviewFaviconIcon } from "./PreviewFaviconIcon"; import type { PreviewableServer } from "./useDiscoveredLocalServers"; interface Props { + threadRef: ScopedThreadRef; server: PreviewableServer; onOpen: () => void; } -export function PreviewLocalServerCard({ server, onOpen }: Props) { +export function PreviewLocalServerCard({ threadRef, server, onOpen }: Props) { const subtitle = describeServer(server); return ( ); } function describeServer(server: PreviewableServer): string { if (server.processName) return server.processName; - if (server.listening) return "Listening"; - if (server.source === "configured") return "Configured"; - return "Recently seen"; -} - -function PulsingDot() { - return ( - - - - - ); -} - -function DimDot() { - return ( - - ); + return "Listening"; } diff --git a/apps/web/src/components/preview/PreviewMoreMenu.tsx b/apps/web/src/components/preview/PreviewMoreMenu.tsx index a98d33304e88..8b7c75cb1d95 100644 --- a/apps/web/src/components/preview/PreviewMoreMenu.tsx +++ b/apps/web/src/components/preview/PreviewMoreMenu.tsx @@ -169,6 +169,7 @@ export function PreviewMoreMenu({ type="button" onClick={callTab(bridge.resetZoom)} aria-label="Reset zoom" + className="[:hover,[data-pressed]]:bg-foreground/10" disabled={tabDisabled} > diff --git a/apps/web/src/components/preview/PreviewPanelShell.test.ts b/apps/web/src/components/preview/PreviewPanelShell.test.ts index 4ac086157a2f..23deb066a2ab 100644 --- a/apps/web/src/components/preview/PreviewPanelShell.test.ts +++ b/apps/web/src/components/preview/PreviewPanelShell.test.ts @@ -1,6 +1,8 @@ +import { jsx } from "react/jsx-runtime"; +import { renderToStaticMarkup } from "react-dom/server"; import { describe, expect, it } from "vite-plus/test"; -import { getPreviewPanelMaxWidth } from "./PreviewPanelShell"; +import { getPreviewPanelMaxWidth, PreviewPanelShell } from "./PreviewPanelShell"; describe("getPreviewPanelMaxWidth", () => { it("allows the panel to use 70% of an ultra-wide viewport without a pixel ceiling", () => { @@ -10,4 +12,38 @@ describe("getPreviewPanelMaxWidth", () => { it("rounds fractional CSS pixels down", () => { expect(getPreviewPanelMaxWidth(2_001)).toBe(1_400); }); + + it("keeps inline panels inside their containing workspace", () => { + const markup = renderToStaticMarkup( + jsx(PreviewPanelShell, { mode: "inline", defaultWidth: 1_000, children: "Panel" }), + ); + + expect(markup).toContain("max-w-full"); + }); + + it("reserves the sibling column minimum when the flex row is known", () => { + // Fullscreen 14" MacBook: viewport 1512, sidebar ~256 → row of 1256. + // The 70% fraction (1058) would leave the chat column only ~198px; + // the container clamp caps the panel at 1256 − 360 instead. + expect(getPreviewPanelMaxWidth(1_512, 1_256)).toBe(896); + }); + + it("keeps the fraction cap when the row is wide enough for both columns", () => { + expect(getPreviewPanelMaxWidth(3_000, 2_900)).toBe(2_100); + }); + + it("rounds fractional row widths down", () => { + expect(getPreviewPanelMaxWidth(1_512, 1_256.6)).toBe(896); + }); + + it("never drops below the panel minimum when the row cannot fit both columns", () => { + // ~1000px window with an expanded sidebar → row of 700. The sibling + // reservation (700 − 360 = 340) would undercut the panel's own 360 + // minimum and invert the resize clamp, so the floor wins. + expect(getPreviewPanelMaxWidth(1_000, 700)).toBe(360); + }); + + it("stays at the panel minimum even when the row is narrower than the reservation", () => { + expect(getPreviewPanelMaxWidth(1_512, 300)).toBe(360); + }); }); diff --git a/apps/web/src/components/preview/PreviewPanelShell.tsx b/apps/web/src/components/preview/PreviewPanelShell.tsx index 6cca09b3a2b8..5c5957f90dd8 100644 --- a/apps/web/src/components/preview/PreviewPanelShell.tsx +++ b/apps/web/src/components/preview/PreviewPanelShell.tsx @@ -1,4 +1,11 @@ -import { type ReactNode, useEffect, useState } from "react"; +import { + type ReactNode, + type RefObject, + useEffect, + useLayoutEffect, + useRef, + useState, +} from "react"; import { isElectron } from "~/env"; import { useResizableWidth } from "~/hooks/useResizableWidth"; @@ -10,12 +17,31 @@ export type PreviewPanelMode = "inline" | "sheet" | "sidebar" | "embedded"; const PREVIEW_PANEL_WIDTH_STORAGE_KEY = "marcode:preview-panel-width"; const PREVIEW_PANEL_MIN_WIDTH = 360; -/** Fraction of the viewport allowed, preserving the remaining space for chat. */ +/** + * Upper bound as a fraction of the viewport; only binds on wide screens. + * On narrow windows the container clamp below is what preserves the + * sibling column's space. + */ const PREVIEW_PANEL_MAX_WIDTH_FRACTION = 0.7; const PREVIEW_PANEL_DEFAULT_WIDTH = 540; +/** + * Width reserved for the sibling column (chat, pull-request list) sharing the + * panel's flex row. The viewport fraction alone is not enough: the app + * sidebar sits outside the row, so on narrow windows (any MacBook, even + * fullscreen) the remaining 30% of the viewport minus the sidebar left the + * sibling below its usable width and the composer overflowed. + */ +const SIBLING_COLUMN_MIN_WIDTH = 360; -export function getPreviewPanelMaxWidth(viewportWidth: number): number { - return Math.floor(viewportWidth * PREVIEW_PANEL_MAX_WIDTH_FRACTION); +export function getPreviewPanelMaxWidth(viewportWidth: number, containerWidth?: number): number { + const fractionCap = Math.floor(viewportWidth * PREVIEW_PANEL_MAX_WIDTH_FRACTION); + const containerCap = + containerWidth === undefined ? Infinity : Math.floor(containerWidth) - SIBLING_COLUMN_MIN_WIDTH; + // Never below the panel's own minimum: when the row cannot fit both + // columns' minimums the sibling yields, and useResizableWidth's clamp + // must not see max < min (it would resolve the inversion to min and, + // via drag-end persistence, overwrite the user's stored width). + return Math.max(PREVIEW_PANEL_MIN_WIDTH, Math.min(fractionCap, containerCap)); } /** @@ -39,7 +65,10 @@ export function PreviewPanelShell(props: { }) { const useDragRegion = isElectron && props.mode !== "sheet" && props.mode !== "embedded"; const isInline = props.mode === "inline"; - const maxWidth = useViewportClampedMaxWidth(); + const hostRef = useRef(null); + // Only inline non-maximized mode applies `width`/`maxWidth`; skip the + // container measurement (and its re-renders) everywhere else. + const maxWidth = useClampedMaxWidth(hostRef, isInline && !props.maximized); const { width, handlers } = useResizableWidth({ storageKey: props.widthStorageKey ?? PREVIEW_PANEL_WIDTH_STORAGE_KEY, defaultWidth: props.defaultWidth ?? PREVIEW_PANEL_DEFAULT_WIDTH, @@ -50,8 +79,9 @@ export function PreviewPanelShell(props: { return (
    , enabled: boolean): number { const [vw, setVw] = useState(() => (typeof window === "undefined" ? 1280 : window.innerWidth)); + const [containerWidth, setContainerWidth] = useState(undefined); useEffect(() => { if (typeof window === "undefined") return; let frame = 0; @@ -93,5 +128,24 @@ function useViewportClampedMaxWidth(): number { if (frame !== 0) window.cancelAnimationFrame(frame); }; }, []); - return getPreviewPanelMaxWidth(vw); + useLayoutEffect(() => { + if (!enabled) return; + const parent = hostRef.current?.parentElement; + if (!parent) return; + // Measure before first paint: the persisted width must be clamped + // against the row on the initial render, not one observer tick later + // (the panel would flash over-wide on every mount). clientWidth is + // integral, so sub-pixel resize deltas bail out of re-rendering. + const measure = () => { + setContainerWidth(parent.clientWidth); + }; + measure(); + if (typeof ResizeObserver === "undefined") return; + const observer = new ResizeObserver(measure); + observer.observe(parent); + return () => { + observer.disconnect(); + }; + }, [hostRef, enabled]); + return getPreviewPanelMaxWidth(vw, containerWidth); } diff --git a/apps/web/src/components/preview/PreviewRecentUrlCard.tsx b/apps/web/src/components/preview/PreviewRecentUrlCard.tsx index 892ff579d1d7..39af63a90616 100644 --- a/apps/web/src/components/preview/PreviewRecentUrlCard.tsx +++ b/apps/web/src/components/preview/PreviewRecentUrlCard.tsx @@ -1,18 +1,20 @@ +import type { ScopedThreadRef } from "@t3tools/contracts"; import { X } from "lucide-react"; import { isValidHistoryTimestamp, type BrowserHistoryEntry } from "~/browserHistoryStore"; import { useNowMinute } from "~/hooks/useNowMinute"; import { formatRelativeTimeLabel } from "~/timestampFormat"; -import { BrowserMockup } from "./BrowserMockup"; +import { PreviewFaviconIcon } from "./PreviewFaviconIcon"; interface Props { + threadRef: ScopedThreadRef; entry: BrowserHistoryEntry; onOpen: () => void; onRemove: () => void; } -export function PreviewRecentUrlCard({ entry, onOpen, onRemove }: Props) { +export function PreviewRecentUrlCard({ threadRef, entry, onOpen, onRemove }: Props) { const parsed = new URL(entry.url); const path = parsed.pathname === "/" ? "" : parsed.pathname; const label = `${parsed.host}${path}${parsed.search}${parsed.hash}`; @@ -27,7 +29,7 @@ export function PreviewRecentUrlCard({ entry, onOpen, onRemove }: Props) { onClick={onOpen} className="flex w-full items-center gap-3 px-3 py-3 pr-10 text-left hover:bg-accent/40 focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring" > - +
    {entry.title ?? label} diff --git a/apps/web/src/components/preview/PreviewView.test.tsx b/apps/web/src/components/preview/PreviewView.test.tsx index d9671e2f2d98..c3fe5337d087 100644 --- a/apps/web/src/components/preview/PreviewView.test.tsx +++ b/apps/web/src/components/preview/PreviewView.test.tsx @@ -1,4 +1,10 @@ -import { EnvironmentId, ThreadId } from "@t3tools/contracts"; +import { + DEFAULT_PREVIEW_APPEARANCE, + DEFAULT_PREVIEW_ZOOM_FACTOR, + EnvironmentId, + FILL_PREVIEW_VIEWPORT, + ThreadId, +} from "@t3tools/contracts"; import { renderToStaticMarkup } from "react-dom/server"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; @@ -41,6 +47,34 @@ vi.mock("~/state/session", () => ({ readPreparedConnection: mocks.readPreparedConnection, })); +// Stubbed at the direct dependency rather than letting the real module pull in +// `useSettings` -> `state/server`, which would drag the whole settings and +// connection graph into a test that only cares about the browser chrome. +vi.mock("~/browser/browserDefaults", () => ({ + useBrowserDefaults: () => ({ + viewport: FILL_PREVIEW_VIEWPORT, + zoomFactor: DEFAULT_PREVIEW_ZOOM_FACTOR, + appearance: DEFAULT_PREVIEW_APPEARANCE, + autoShowFloatingPreview: true, + }), + getBrowserDefaults: () => ({ + viewport: FILL_PREVIEW_VIEWPORT, + zoomFactor: DEFAULT_PREVIEW_ZOOM_FACTOR, + appearance: DEFAULT_PREVIEW_APPEARANCE, + autoShowFloatingPreview: true, + }), + browserDefaultOpenViewport: () => FILL_PREVIEW_VIEWPORT, + browserDefaultTabState: () => ({ + zoomFactor: DEFAULT_PREVIEW_ZOOM_FACTOR, + colorScheme: DEFAULT_PREVIEW_APPEARANCE, + }), + browserResponsiveViewportForToggle: () => ({ + _tag: "freeform" as const, + width: 1024, + height: 768, + }), +})); + vi.mock("~/composerDraftStore", () => ({ useComposerDraftStore: ( select: (store: { addPreviewAnnotation: () => void; addImage: () => void }) => unknown, diff --git a/apps/web/src/components/preview/PreviewView.tsx b/apps/web/src/components/preview/PreviewView.tsx index 6979a1a4006d..5a828b863ced 100644 --- a/apps/web/src/components/preview/PreviewView.tsx +++ b/apps/web/src/components/preview/PreviewView.tsx @@ -43,7 +43,7 @@ import { commitBrowserViewportChange, subscribeBrowserViewportChange, } from "~/browser/browserViewportActions"; -import { resolveResponsiveBrowserViewportSize } from "~/browser/browserViewportLayout"; +import { browserResponsiveViewportForToggle, useBrowserDefaults } from "~/browser/browserDefaults"; import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId"; import { PreviewUnreachable } from "./PreviewUnreachable"; import { revealInFileExplorerLabel } from "./fileExplorerLabel"; @@ -144,6 +144,7 @@ export function PreviewView({ const controller = desktopOverlay?.controller ?? "none"; const loadProgress = useLoadingProgress(loading); const viewport = snapshot?.viewport ?? FILL_PREVIEW_VIEWPORT; + const browserDefaults = useBrowserDefaults(); const panelRect = useBrowserSurfaceStore((state) => runtimeTabId ? (state.byTabId[runtimeTabId]?.rect ?? null) : null, ); @@ -249,12 +250,14 @@ export function PreviewView({ return; } - const responsiveSize = panelRect - ? resolveResponsiveBrowserViewportSize(panelRect, desktopOverlay?.zoomFactor) - : { width: 1024, height: 768 }; - void commitBrowserViewportChange(runtimeTabId, { _tag: "freeform", ...responsiveSize }).catch( - () => undefined, - ); + void commitBrowserViewportChange( + runtimeTabId, + browserResponsiveViewportForToggle({ + defaults: browserDefaults, + panelRect, + zoomFactor: desktopOverlay?.zoomFactor, + }), + ).catch(() => undefined); }; useEffect(() => { @@ -710,9 +713,9 @@ export function PreviewView({ ) : null} {showEmptyState ? ( removeUrlForThread(threadRef, url)} onOpenUrl={(next) => void handleOpenServerUrl(next)} diff --git a/apps/web/src/components/preview/ThreadPreviewMiniPlayer.tsx b/apps/web/src/components/preview/ThreadPreviewMiniPlayer.tsx index 3e7c46ef0e0a..623928d102ef 100644 --- a/apps/web/src/components/preview/ThreadPreviewMiniPlayer.tsx +++ b/apps/web/src/components/preview/ThreadPreviewMiniPlayer.tsx @@ -2,12 +2,13 @@ import type { ScopedThreadRef } from "@t3tools/contracts"; import { PanelRightIcon, PictureInPicture2, XIcon } from "lucide-react"; -import { type PointerEvent as ReactPointerEvent, useLayoutEffect, useRef } from "react"; +import { type PointerEvent as ReactPointerEvent, useLayoutEffect, useRef, useState } from "react"; import { BrowserSurfaceSlot } from "~/browser/BrowserSurfaceSlot"; import { previewRuntimeTabId } from "~/browser/previewRuntimeTabId"; import { Button } from "~/components/ui/button"; import { toastManager } from "~/components/ui/toast"; +import { Tooltip, TooltipPopup, TooltipTrigger } from "~/components/ui/tooltip"; import { useThreadPreviewState } from "~/previewStateStore"; import { selectThreadPreviewMiniPlayer, usePreviewMiniPlayerStore } from "~/previewMiniPlayerStore"; import { useRightPanelStore } from "~/rightPanelStore"; @@ -17,6 +18,7 @@ import { clampPreviewMiniPlayerPosition, clampPreviewMiniPlayerSize, PREVIEW_MINI_PLAYER_DEFAULT_SIZE, + PREVIEW_MINI_PLAYER_EDGE_GAP, } from "./previewMiniPlayerLayout"; interface DragState { @@ -31,6 +33,8 @@ interface ResizeState { readonly pointerId: number; readonly pointerX: number; readonly pointerY: number; + readonly playerX: number; + readonly playerY: number; readonly width: number; readonly height: number; } @@ -45,6 +49,7 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props const rootRef = useRef(null); const dragRef = useRef(null); const resizeRef = useRef(null); + const [defaultLayoutVersion, setDefaultLayoutVersion] = useState(""); const miniPlayer = usePreviewMiniPlayerStore((state) => selectThreadPreviewMiniPlayer(state.byThreadKey, threadRef), ); @@ -91,8 +96,12 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props bottomInset, ); usePreviewMiniPlayerStore.getState().resize(threadRef, tabId, nextSize); + if (!position) { + setDefaultLayoutVersion(`${parent.clientWidth}:${parent.clientHeight}`); + return; + } const next = clampPreviewMiniPlayerPosition( - position ?? { x: root.offsetLeft, y: root.offsetTop }, + position, { width: parent.clientWidth, height: parent.clientHeight }, nextSize, bottomInset, @@ -159,11 +168,16 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props const handleResizePointerDown = (event: ReactPointerEvent) => { if (event.button !== 0) return; const root = rootRef.current; - if (!root) return; + const parent = root?.offsetParent; + if (!root || !(parent instanceof HTMLElement)) return; + const rootRect = root.getBoundingClientRect(); + const parentRect = parent.getBoundingClientRect(); resizeRef.current = { pointerId: event.pointerId, pointerX: event.clientX, pointerY: event.clientY, + playerX: rootRect.left - parentRect.left, + playerY: rootRect.top - parentRect.top, width: root.offsetWidth, height: root.offsetHeight, }; @@ -194,7 +208,7 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props ); usePreviewMiniPlayerStore.getState().resize(threadRef, tabId, nextSize); const nextPosition = clampPreviewMiniPlayerPosition( - position ?? { x: root.offsetLeft, y: root.offsetTop }, + { x: resize.playerX, y: resize.playerY }, { width: parent.clientWidth, height: parent.clientHeight }, nextSize, bottomInset, @@ -222,8 +236,8 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props position ? { left: position.x, top: position.y, width: size.width, height: size.height } : { - right: 16, - top: 16, + right: PREVIEW_MINI_PLAYER_EDGE_GAP, + top: PREVIEW_MINI_PLAYER_EDGE_GAP, width: size.width, height: size.height, } @@ -241,45 +255,63 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props onPointerUp={endDrag} onPointerCancel={endDrag} > - - - + : "Pop into separate window"} + + + + event.stopPropagation()} + onClick={close} + /> + } + > + + + Close floating preview +
    @@ -290,7 +322,11 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props visible={Boolean(desktopOverlay?.hasWebContents)} cornerRadius={12} fitSourceContent - layoutVersion={position ? `${position.x}:${position.y}` : `initial:${bottomInset}`} + layoutVersion={ + position + ? `${position.x}:${position.y}` + : `initial:${bottomInset}:${defaultLayoutVersion}` + } className="absolute inset-0" />
    @@ -302,7 +338,6 @@ export function ThreadPreviewMiniPlayer({ threadRef, tabId, bottomInset }: Props + )} + + ))} + + ); +} + +/** + * The checks indicator and what it opens, in both places a change request is shown: a listing + * row, which knows only the rollup, and the detail header, which is already holding every check. + * + * `checks` decides between the two. Given them, nothing is read; without them, the popup reads + * the detail itself, which is why the row must also say which environment it came from. + */ +export function PullRequestChecksPopover({ + checksState, + checks, + environmentId, + reference, + className, +}: { + checksState: PullRequestChecksState; + /** The checks already in hand, for the detail header. Absent on a listing row. */ + checks?: ReadonlyArray; + environmentId?: EnvironmentId; + reference?: PullRequestRef; + className?: string; +}) { + const presentation = pullRequestChecksStatePresentation(checksState); + // Counts beat the rollup's own wording where they are known, the way GitHub's own header reads. + const summary = checks === undefined ? null : summarizePullRequestChecks(checks); + return ( + + {/* A listing row is itself a button, so the trigger renders as a span: a nested button is + not valid inside one. The click is stopped here so opening the checks does not also + select the row it sits on. */} + + } + onClick={(event) => event.stopPropagation()} + > + + + +

    {presentation.label}

    + {summary === null ? null :

    {summary}

    } + {checks !== undefined ? ( + + ) : environmentId !== undefined && reference !== undefined ? ( + + ) : null} +     +     + ); +} diff --git a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx index 66ac8b8196ae..a2e36f708ef7 100644 --- a/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx +++ b/apps/web/src/components/pullRequest/PullRequestCodeTab.tsx @@ -4,8 +4,11 @@ import type { EnvironmentId, PullRequestDetailView, PullRequestDiffSide, + PullRequestOmittedFileStat, PullRequestRef, + PullRequestReviewPosition, PullRequestReviewThread, + PullRequestThreadCommentsResult, } from "@t3tools/contracts"; import { ChevronDownIcon, @@ -16,7 +19,6 @@ import { MessageSquareIcon, MessageSquareOffIcon, Rows3Icon, - SparklesIcon, TextWrapIcon, TriangleAlertIcon, XIcon, @@ -28,6 +30,8 @@ import { useClientSettings } from "~/hooks/useSettings"; import { useTheme } from "~/hooks/useTheme"; import { areAllDiffFilesCollapsed } from "~/lib/diffCollapse"; import { pullRequestFindingKey, type PullRequestFinding } from "./pullRequestDetail.logic"; +import { canEditPullRequestComment } from "./pullRequestEditing.logic"; +import { orderDiffFiles } from "./pullRequestFileOrder.logic"; import { buildFileDiffRenderKey, fnv1a32, @@ -40,7 +44,11 @@ import { } from "~/lib/diffRendering"; import { cn } from "~/lib/utils"; import { createPullRequestDiffFileContentsLoader } from "~/lib/diffFileContents"; -import { buildDiffReviewComment, type ReviewCommentContext } from "~/reviewCommentContext"; +import { + buildDiffReviewComment, + resolveDiffReviewPosition, + type ReviewCommentContext, +} from "~/reviewCommentContext"; import { pullRequestEnvironment } from "~/state/pullRequests"; import { useEnvironmentQuery } from "~/state/query"; import { useAtomCommand } from "~/state/use-atom-command"; @@ -95,8 +103,20 @@ interface DiffSlice { readonly patch: string; readonly truncated: boolean; readonly nextCursor: string | null; + readonly omittedFileStats: ReadonlyArray; } +/** + * The viewer's own per-file counts are hidden and drawn from this side of its shadow root + * instead: its counts are hunk sums, and a file whose hunks the host withheld would read as + * an empty change rather than as the counts the host did report. + */ +const REPLACE_FILE_COUNTS_CSS = ` +[data-diffs-header] [data-additions-count], +[data-diffs-header] [data-deletions-count] { + display: none !important; +}`; + /** Nothing loaded yet, as one identity, so the memos below do not see a new array every render. */ const NO_SLICES: ReadonlyArray = []; @@ -114,18 +134,16 @@ interface DraftAnchor { readonly path: string; /** What the file was called before the change, for the hosts that resolve a position by both. */ readonly oldPath: string | null; - readonly line: number; - readonly side: PullRequestDiffSide; + readonly position: PullRequestReviewPosition; /** The whole selection, which the comment collapses to one line but a question keeps. */ readonly range: SelectedLineRange; } -/** A range of the diff, and whatever the reader wants to know about it. */ -export interface PullRequestAskSelectionInput { +/** A range of the diff and the reader's request for the agent. */ +export interface PullRequestAgentSelectionInput { /** The marked lines, already in the shape the composer draws and the agent reads. */ readonly comment: ReviewCommentContext; - /** Empty where the reader pressed Ask without typing: the lines are the question. */ - readonly question: string; + readonly request: string; } /** The contract's sides named the way the diff viewer names them, and back again. */ @@ -133,8 +151,21 @@ function toViewerSide(side: PullRequestDiffSide) { return side === "left" ? ("deletions" as const) : ("additions" as const); } -function fromViewerSide(side: string | undefined): PullRequestDiffSide { - return side === "deletions" ? "left" : "right"; +function getReviewPositionAnchor(position: PullRequestReviewPosition): { + line: number; + side: PullRequestDiffSide; +} { + switch (position.kind) { + case "added": + return { line: position.newLine, side: "right" }; + case "deleted": + return { line: position.oldLine, side: "left" }; + case "context": + return { + line: position.side === "left" ? position.oldLine : position.newLine, + side: position.side, + }; + } } /** @@ -155,8 +186,9 @@ export function PullRequestCodeTab({ selectedCommitOid, onSelectedCommitChange, pendingFinding, + fixFindingLabel = "Fix in a thread", onFixFinding, - onAskAboutSelection, + onAddToAgentSelection, onRefresh, refreshToken = 0, }: { @@ -168,9 +200,10 @@ export function PullRequestCodeTab({ onSelectedCommitChange: (oid: string | null) => void; /** The hand-off currently preparing, if any, so only the finding it belongs to says so. */ pendingFinding?: string | null; + fixFindingLabel?: string; onFixFinding?: (finding: PullRequestFinding) => void; - /** Absent where a selection has no agent to go to, which takes the Ask button off the box. */ - onAskAboutSelection?: (input: PullRequestAskSelectionInput) => void; + /** Absent where there is no active agent composer to receive a local comment. */ + onAddToAgentSelection?: (input: PullRequestAgentSelectionInput) => void; onRefresh: () => void; /** Bumped by the panel's refresh button: drop the accumulated pages and re-read the diff. */ refreshToken?: number; @@ -246,6 +279,7 @@ export function PullRequestCodeTab({ patch: data.patch, truncated: data.truncated, nextCursor: data.nextCursor, + omittedFileStats: data.omittedFileStats ?? [], }; const index = slices.findIndex((slice) => slice.cursor === cursor); if (index === -1) { @@ -256,7 +290,17 @@ export function PullRequestCodeTab({ existing !== undefined && existing.patch === next.patch && existing.truncated === next.truncated && - existing.nextCursor === next.nextCursor + existing.nextCursor === next.nextCursor && + existing.omittedFileStats.length === next.omittedFileStats.length && + existing.omittedFileStats.every((file, index) => { + const refreshed = next.omittedFileStats[index]; + return ( + refreshed !== undefined && + refreshed.path === file.path && + refreshed.additions === file.additions && + refreshed.deletions === file.deletions + ); + }) ) { return previous; } @@ -290,6 +334,12 @@ export function PullRequestCodeTab({ const setThreadResolution = useAtomCommand(pullRequestEnvironment.setThreadResolution, { reportFailure: false, }); + const updateComment = useAtomCommand(pullRequestEnvironment.updateComment, { + reportFailure: false, + }); + const loadThreadComments = useAtomCommand(pullRequestEnvironment.threadComments, { + reportFailure: false, + }); const getDiffFileContents = useAtomCommand(pullRequestEnvironment.diffFileContents); const loadDiffFiles = useMemo( () => @@ -337,16 +387,12 @@ export function PullRequestCodeTab({ }), [loadedSlices, resolvedTheme, scopeKey], ); - // Sorted within a slice rather than across them: sorting the accumulated set would let a late + // Ordered within a slice rather than across them: ordering the accumulated set would let a late // slice push a file the reader is part way through further down the page. const files = useMemo( () => parsedSlices.flatMap((parsed) => - parsed?.kind === "files" - ? parsed.files.toSorted((left, right) => - resolveFileDiffPath(left).localeCompare(resolveFileDiffPath(right)), - ) - : [], + parsed?.kind === "files" ? orderDiffFiles(parsed.files) : [], ), [parsedSlices], ); @@ -415,10 +461,14 @@ export function PullRequestCodeTab({ if (commit === null) { for (const comment of pendingComments) { if (comment.path !== path) continue; - groupAt(comment.side, comment.line).pending.push(comment); + const anchor = getReviewPositionAnchor(comment.position); + groupAt(anchor.side, anchor.line).pending.push(comment); } } - if (draft?.fileKey === fileKey) groupAt(draft.side, draft.line).draft = true; + if (draft?.fileKey === fileKey) { + const anchor = getReviewPositionAnchor(draft.position); + groupAt(anchor.side, anchor.line).draft = true; + } const collapsed = isFileDiffCollapsed(fileKey, foldOverride, toggledFiles); @@ -449,7 +499,13 @@ export function PullRequestCodeTab({ }:${thread.comments .map( (comment) => - `${comment.id}:${comment.author?.login ?? ""}:${comment.createdAt}:${comment.body}`, + `${comment.id}:${comment.author?.login ?? ""}:${comment.createdAt}:${comment.body}:${( + comment.reactions ?? [] + ) + .map( + (r) => `${r.content}:${r.count}:${r.viewerHasReacted ? "v" : ""}`, + ) + .join(",")}`, ) .join(";")}`, ) @@ -471,6 +527,15 @@ export function PullRequestCodeTab({ ], ); const lineStat = useMemo(() => getDiffLineStat(files), [files]); + const omittedFileStats = useMemo( + () => + new Map( + loadedSlices.flatMap((slice) => + slice.omittedFileStats.map((file) => [file.path, file] as const), + ), + ), + [loadedSlices], + ); const fileKeys = useMemo(() => items.map((item) => item.id), [items]); const collapsedFileKeys = useMemo( () => new Set(items.filter((item) => item.collapsed === true).map((item) => item.id)), @@ -552,12 +617,13 @@ export function PullRequestCodeTab({ // that silently lost its first line on the other hosts would be worse than one line. const path = resolveFileDiffPath(file); const previousPath = resolveFileDiffPreviousPath(file); + const position = resolveDiffReviewPosition(file, range.end, range.endSide ?? range.side); + if (position === null) return; setDraft({ fileKey: item.id, path, oldPath: previousPath === path ? null : previousPath, - line: range.end, - side: fromViewerSide(range.endSide ?? range.side), + position, range, }); }, @@ -567,8 +633,8 @@ export function PullRequestCodeTab({ // Built here because the parsed diff only lives here, and built by the same function the // thread panel's own line selection uses — the gesture is the same one, so a second reading of // the hunks would only be a second place for it to drift. - const askAboutSelection = useCallback( - (anchor: DraftAnchor, question: string) => { + const finishSelection = useCallback( + (anchor: DraftAnchor, text: string, onFinish: (comment: ReviewCommentContext) => void) => { const file = files.find((candidate) => buildFileDiffRenderKey(candidate) === anchor.fileKey); const comment = file === undefined @@ -580,14 +646,13 @@ export function PullRequestCodeTab({ filePath: anchor.path, fileDiff: file, range: anchor.range, - text: question, + text, }); setDraft(null); setSelectedLines(null); - if (comment === null || !onAskAboutSelection) return; - onAskAboutSelection({ comment, question }); + if (comment !== null) onFinish(comment); }, - [detail.number, files, onAskAboutSelection], + [detail.number, files], ); // The viewer's SlotPortals memoizes each visible file's header/annotation portal on these @@ -629,13 +694,12 @@ export function PullRequestCodeTab({ { event.stopPropagation(); toggleFile(item.id); @@ -656,6 +720,30 @@ export function PullRequestCodeTab({ [toggleFile], ); + const renderHeaderMetadata = useCallback( + (item: CodeViewItem) => { + if (item.type !== "diff") return null; + let additions = 0; + let deletions = 0; + for (const hunk of item.fileDiff.hunks) { + additions += hunk.additionLines; + deletions += hunk.deletionLines; + } + if (additions === 0 && deletions === 0) { + const withheld = omittedFileStats.get(resolveFileDiffPath(item.fileDiff)); + if (withheld) ({ additions, deletions } = withheld); + } + return ( + + ); + }, + [omittedFileStats], + ); + const diffViewOptions = useMemo( () => ({ diffStyle: diffRenderMode === "split" ? ("split" as const) : ("unified" as const), @@ -706,14 +794,35 @@ export function PullRequestCodeTab({ const renderThreadCard = useCallback( (thread: PullRequestReviewThread) => ( onFixFinding({ kind: "thread", thread }) } : {})} + onLoadMore={async (cursor): Promise => { + const result = await loadThreadComments({ + environmentId, + input: { ...reference, threadId: thread.id, cursor }, + }); + if (result._tag === "Failure") { + toastManager.add({ + type: "error", + title: "More comments could not be loaded", + }); + return null; + } + return result.value; + }} onReply={(body) => runThreadCommand("Reply could not be posted", () => replyToThread({ @@ -722,6 +831,18 @@ export function PullRequestCodeTab({ }), ) } + // A conversation on a line is made of review comments, whatever the host filed them as. + canEditComment={(comment) => + canEditPullRequestComment(detail, { author: comment.author, kind: "review-comment" }) + } + onEditComment={(commentId, body) => + runThreadCommand("The comment could not be saved", () => + updateComment({ + environmentId, + input: { ...reference, commentId, kind: "review-comment", body }, + }), + ) + } onToggleResolved={() => void runThreadCommand("The conversation could not be updated", () => setThreadResolution({ @@ -730,11 +851,15 @@ export function PullRequestCodeTab({ }), ) } + onReacted={onRefresh} /> ), [ - detail.workspaceRoot, + detail, environmentId, + fixFindingLabel, + loadThreadComments, + onRefresh, onFixFinding, pendingFinding, reference, @@ -744,12 +869,13 @@ export function PullRequestCodeTab({ runThreadCommand, setThreadResolution, threadPending, + updateComment, ], ); const renderAnnotation = useCallback( (annotation: ReviewAnnotation) => ( -
    +
    {annotation.metadata.threads.map(renderThreadCard)} {annotation.metadata.pending.map((comment) => ( , - allowEmpty: true, - onAction: (question: string) => askAboutSelection(draft, question), + label: "Add to agent", + onAction: (text: string) => + finishSelection(draft, text, (comment) => + onAddToAgentSelection({ comment, request: text }), + ), }, } : {})} @@ -783,8 +910,7 @@ export function PullRequestCodeTab({ id: nextPendingReviewCommentId(), path: draft.path, ...(draft.oldPath === null ? {} : { oldPath: draft.oldPath }), - line: draft.line, - side: draft.side, + position: draft.position, body, }); setDraft(null); @@ -796,9 +922,9 @@ export function PullRequestCodeTab({ ), [ addComment, - askAboutSelection, draft, - onAskAboutSelection, + finishSelection, + onAddToAgentSelection, removeComment, renderThreadCard, reviewKey, @@ -817,7 +943,7 @@ export function PullRequestCodeTab({ review.verdicts.length === 0 ? null : (
    {reviewOpen ? ( -
    +
    setReviewOpen(true)} + size="compact" + variant="glass" > Review @@ -860,7 +987,7 @@ export function PullRequestCodeTab({ {pendingComments.length} ) : null} - + )}
    ); @@ -884,7 +1011,7 @@ export function PullRequestCodeTab({ * diff API offers it. */ const toolbar = ( -
    +
    {/* A host that reports no commits has nothing to scope by, and a dropdown whose only entry is the scope already showing is a control that does nothing. */} @@ -912,9 +1039,12 @@ export function PullRequestCodeTab({ > {/* Headlines run long, and the abbreviated oid after one is what a reader matches against the commit list on the host. */} - - {entry.messageHeadline} - + + {entry.messageHeadline}} + /> + {entry.messageHeadline} + {entry.oid.slice(0, 7)} @@ -1180,9 +1310,14 @@ export function PullRequestCodeTab({
    {[...orphanFiles].map(([path, threads]) => (
    -

    - {path} -

    + + {path}

    + } + /> + {path} +
    {threads.map((thread) => (
    @@ -1246,7 +1381,9 @@ export function PullRequestCodeTab({ // is running out of diff. renderCodeViewFooter={renderCodeViewFooter} renderHeaderPrefix={renderHeaderPrefix} + renderHeaderMetadata={renderHeaderMetadata} renderAnnotation={renderAnnotation} + unsafeCSSExtra={REPLACE_FILE_COUNTS_CSS} /> {reviewOverlay}
    diff --git a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx index 426ea15526d5..61414076fce5 100644 --- a/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx +++ b/apps/web/src/components/pullRequest/PullRequestDetailPanel.tsx @@ -1,11 +1,13 @@ -import { scopeProjectRef } from "@t3tools/client-runtime/environment"; +import { scopedThreadKey, scopeProjectRef } from "@t3tools/client-runtime/environment"; import { squashAtomCommandFailure } from "@t3tools/client-runtime/state/runtime"; import type { EnvironmentId, PullRequestAction, PullRequestMergeMethod, + PullRequestUpdateMethod, PullRequestRef, PullRequestState, + ScopedThreadRef, } from "@t3tools/contracts"; import { ArrowDownUpIcon, @@ -28,12 +30,15 @@ import { LinkIcon, MoreHorizontalIcon, PanelRightIcon, + PencilIcon, RefreshCwIcon, + ServerIcon, TriangleAlertIcon, } from "lucide-react"; import { lazy, Suspense, + type MouseEvent as ReactMouseEvent, useCallback, useEffect, useLayoutEffect, @@ -49,6 +54,8 @@ import { usePreparePullRequestThreadAction } from "~/lib/sourceControlActions"; import { cn } from "~/lib/utils"; import { readLocalApi } from "~/localApi"; import type { ReviewCommentContext } from "~/reviewCommentContext"; +import { useProjects } from "~/state/entities"; +import { useEnvironments } from "~/state/environments"; import { useEnvironmentQuery } from "~/state/query"; import { useLiveRefresh } from "~/hooks/useLiveRefresh"; import { pullRequestEnvironment } from "~/state/pullRequests"; @@ -66,6 +73,7 @@ import { } from "../ui/alert-dialog"; import { Badge } from "../ui/badge"; import { Button } from "../ui/button"; +import { Input } from "../ui/input"; import { Menu, MenuItem, @@ -75,17 +83,19 @@ import { MenuSeparator, MenuTrigger, } from "../ui/menu"; +import { Popover, PopoverPopup, PopoverTrigger } from "../ui/popover"; import { toastManager } from "../ui/toast"; import { Tooltip, TooltipPopup, TooltipTrigger } from "../ui/tooltip"; import { PullRequestDetailGhost, PullRequestTimelineGhost } from "./PullRequestGhosts"; import { PullRequestActivityUnavailableState } from "./PullRequestActivityUnavailableState"; import { DiffPanelLoadingState } from "../DiffPanelShell"; import { PullRequestsUnavailableState } from "./PullRequestsUnavailableState"; -import type { PullRequestAskSelectionInput } from "./PullRequestCodeTab"; +import type { PullRequestAgentSelectionInput } from "./PullRequestCodeTab"; +import { openOnHostLabel, showPullRequestLinkContextMenu } from "./pullRequestLinkContextMenu"; import { PullRequestSummaryTab } from "./PullRequestSummaryTab"; import { PullRequestTimelineTab } from "./PullRequestTimelineTab"; import { - buildAskAboutLinesHandoff, + buildAddSelectionToAgentHandoff, buildAskAboutPullRequestHandoff, buildExplainPullRequestHandoff, buildFixFindingHandoff, @@ -93,14 +103,30 @@ import { buildResolveConflictsPrompt, handoffPrompt, handoffReviewComments, + latestPullRequestReviewOutcomes, + pullRequestActionMenuHasGroup, + pullRequestActionNeedsHostRefresh, + pullRequestComposerTarget, pullRequestFindingKey, + pullRequestHandoffLabels, readableFailure, + resolveBaseFreshness, type PullRequestFinding, + shouldRefreshPullRequestActivity, } from "./pullRequestDetail.logic"; +import { canEditPullRequestChangeRequest } from "./pullRequestEditing.logic"; +import { + resolvePickableEnvironments, + type PickableEnvironment, +} from "./pullRequestProjectAssignment.logic"; +import { PullRequestChecksPopover } from "./PullRequestChecksPopover"; import { PullRequestActorLabel, PullRequestDiffStat, PullRequestMetaLine, + PullRequestReviewOutcomeIcon, + pullRequestChecksState, + pullRequestReviewOutcomeToneClassName, resolvePullRequestState, summarizePullRequestChecks, } from "./pullRequestPresentation"; @@ -113,6 +139,12 @@ const ACTION_SUCCESS_LABELS: Record = { draft: "Converted to draft", close: "Pull request closed", reopen: "Pull request reopened", + "update-branch": "Branch updated with the base branch", + // True whichever it did: a pull request that was already mergeable merges the moment this is + // armed, and the client has no way to tell that apart from one still waiting on something. + "enable-auto-merge": + "Auto-merge turned on — merges as soon as this is ready, sooner if it already is", + "disable-auto-merge": "Auto-merge turned off", }; /** Said as the thing that did not happen, rather than as the operation that returned an error. */ @@ -122,6 +154,9 @@ const ACTION_FAILURE_LABELS: Record = { draft: "Could not convert this to a draft", close: "Could not close this pull request", reopen: "Could not reopen this pull request", + "update-branch": "Could not update this branch", + "enable-auto-merge": "Could not turn on auto-merge", + "disable-auto-merge": "Could not turn off auto-merge", }; /** What to try, for the times the host says only that it refused. */ @@ -133,15 +168,25 @@ const ACTION_FAILURE_HINTS: Record = { close: "The host refused it. Check that you have write access, or that you opened it.", reopen: "The host refused it. Check that you have write access, and that the branch still exists.", + // Said for the merge commit, which is what an update is unless a rebase was asked for. The + // rebase has its own reasons to fail and its own sentence below. + "update-branch": + "The host refused it. Check that you have write access to the branch — one from a fork also needs its author to allow edits from maintainers — and that it does not conflict with the base.", + // The one refusal that is usually a repository setting rather than anything about this branch: + // GitHub will not arm an auto-merge at all unless the repository has the feature switched on. + "enable-auto-merge": + "The host refused it. Check that this repository allows auto-merge, that you have write access, and that there is something left for it to wait on.", + "disable-auto-merge": + "The host refused it. Check that you have write access, and that the merge has not already happened.", }; -/** Named for the host rather than "externally": the point is where you will land. */ -const OPEN_ON_HOST_LABELS: Partial> = { - github: "Open on GitHub", - gitlab: "Open on GitLab", - bitbucket: "Open on Bitbucket", - "azure-devops": "Open on Azure DevOps", -}; +/** + * Said instead of the update hint when the reader asked for a rebase: it is the one that fails on + * its own merits, because GitHub replays the commits and stops at the first that does not apply. + * Offering the merge commit only makes sense to somebody who did not already choose it. + */ +const UPDATE_BRANCH_REBASE_FAILURE_HINT = + "The host refused it. A rebase stops at the first commit that does not apply cleanly; updating with a merge commit may still work."; const TABS: ReadonlyArray<{ value: DetailTab; label: string }> = [ { value: "summary", label: "Summary" }, @@ -160,7 +205,148 @@ const PullRequestCodeTab = lazy(loadCodeTab); * is closed by the time the next one opens. It is how a prompt the reader has since edited is told * apart from the one they were handed: only the sentence still exactly as written may be replaced. */ -const lastHandoffPromptByDraft = new Map(); +const lastHandoffPromptByDraft = new Map(); + +const composerTargetKey = (target: ScopedThreadRef | DraftId): string => + typeof target === "string" ? target : scopedThreadKey(target); + +/** + * Which server the checkout and the hand-offs land on, where more than one of them holds this + * repository. The list picked one of them to show the pull request under, so that everything on + * it is read from somewhere; where the reader wants to work is a separate answer, and this is + * where they give it. + */ +function ActOnEnvironmentPicker({ + environments, + value, + onChange, + disabled, +}: { + environments: ReadonlyArray; + value: EnvironmentId; + onChange: (environmentId: EnvironmentId) => void; + disabled: boolean; +}) { + return ( + <> + + onChange(environmentId as EnvironmentId)} + > + {environments.map((environment) => ( + + {/* The radio item lays its children out as one block, so the icon and the label + need their own row to share a line. */} + + + {environment.label} + + + ))} + + + ); +} + +/** The number is a link in every place the host writes it, so the right-click that copies one + has to answer here too — otherwise the platform's own cut/paste menu opens over it. */ +const openNumberContextMenu = ( + event: ReactMouseEvent, + detail: { readonly url: string; readonly provider: string }, +): void => { + event.preventDefault(); + event.stopPropagation(); + void showPullRequestLinkContextMenu({ + url: detail.url, + openLabel: openOnHostLabel(detail.provider), + position: { x: event.clientX, y: event.clientY }, + }); +}; + +/** + * The stale-branch warning, said beside the branch it is about rather than as a bar of its own. + * The banner this replaces held a row of chrome open across the top of every pull request that + * had fallen behind, pushing the reading down to say something that is true of the base branch + * and nothing else; as a mark on the base branch it is where a reader would look for it, and the + * sentence and the way out of it arrive together the moment the mark is pointed at. + * + * A popover rather than a tooltip because what it holds can be pressed: a tooltip's layer takes + * no pointer, and a control nobody can reach is worse than no control. + */ +function PullRequestBaseFreshnessWarning({ + baseBranch, + freshness, + pending, + onUpdate, + iconClassName, +}: { + readonly baseBranch: string; + readonly freshness: { + readonly behindBy: number | null; + readonly methods: ReadonlyArray; + }; + readonly pending: boolean; + readonly onUpdate: (method: PullRequestUpdateMethod) => void; + readonly iconClassName?: string; +}) { + const behind = + freshness.behindBy === null + ? "" + : ` by ${freshness.behindBy.toLocaleString()} ${ + freshness.behindBy === 1 ? "commit" : "commits" + }`; + const summary = `This branch is out-of-date with ${baseBranch}${behind}.`; + return ( + + + } + > + + + +

    {summary}

    +

    Changes can be cleanly merged.

    + {/* Each way the host offers and this reader may take, as its own button: a split button + would need a menu inside a popover, and two buttons say the same thing in one layer. */} + {freshness.methods.length > 0 ? ( + + {freshness.methods.map((method) => ( + + ))} + + ) : null} +     +     + ); +} export function PullRequestDetailPanel({ environmentId, @@ -171,6 +357,7 @@ export function PullRequestDetailPanel({ onStateChange, context = "page", chromeVariant = "full", + composerDraftTarget, }: { environmentId: EnvironmentId; reference: PullRequestRef; @@ -207,6 +394,11 @@ export function PullRequestDetailPanel({ * top — the chrome spends its height on what is being read. */ chromeVariant?: "full" | "collapse"; + /** + * The open thread's composer. Beside the thread whose own pull request this is, hand-offs + * land here instead of opening a new thread — the branch is already under the reader's feet. + */ + composerDraftTarget?: ScopedThreadRef | DraftId; }) { const pullRequestKey = `${reference.projectId}:${reference.repository}#${reference.number}`; const [tab, setTab] = useState("summary"); @@ -268,7 +460,9 @@ export function PullRequestDetailPanel({ if (scroller) scroller.scrollTop = Math.max(0, scroller.scrollTop + delta); }, [condensed]); const [mergeMethod, setMergeMethod] = useState("merge"); - const [confirmAction, setConfirmAction] = useState<"merge" | "close" | null>(null); + const [confirmAction, setConfirmAction] = useState< + "merge" | "close" | "enable-auto-merge" | null + >(null); // Which handoff is preparing, keyed so a per-finding button can say "Preparing..." on itself // alone. One at a time whatever the key: they all check the same pull request out. const [handoff, setHandoff] = useState(null); @@ -311,6 +505,7 @@ export function PullRequestDetailPanel({ commentsTruncated: activity?.commentsTruncated ?? false, reviewThreads: activity?.reviewThreads ?? [], commits: activity?.commits ?? [], + reactions: activity?.reactions ?? [], }, [activity, coreDetail], ); @@ -320,6 +515,17 @@ export function PullRequestDetailPanel({ detailQuery.refresh(); activityQuery.refresh(); }, [activityQuery.refresh, detailQuery.refresh]); + const activityRevision = useRef<{ readonly key: string; readonly updatedAt: string } | null>( + null, + ); + useEffect(() => { + if (!coreDetail) return; + const next = { key: pullRequestKey, updatedAt: coreDetail.updatedAt }; + if (shouldRefreshPullRequestActivity(activityRevision.current, next)) { + activityQuery.refresh(); + } + activityRevision.current = next; + }, [activityQuery.refresh, coreDetail, pullRequestKey]); useEffect(() => { if (!detail) return; onStateChange?.({ @@ -330,11 +536,11 @@ export function PullRequestDetailPanel({ isDraft: detail.isDraft, }); }, [detail, onStateChange]); - // A pull request changes while it is open in front of somebody — a push lands, a check - // finishes, a review arrives — so the panel reads it again on the way back to the window and - // while a reader sits on it. Keyed by the pull request rather than by the panel, because this - // one panel shows a different pull request every time it is opened. - useLiveRefresh(refreshDetail, { + // Core detail is cheap enough to re-read while this stays open. Activity is heavier, so the + // revision effect above reads it only after this same pull request reports a change. Keyed by + // the pull request rather than by the panel, because this one panel shows a different pull + // request every time it is opened. + useLiveRefresh(detailQuery.refresh, { key: `pull-request:${reference.projectId}:${reference.repository}#${reference.number}`, }); // The button, on the other hand, goes around the server's cache rather than through it: it is @@ -356,44 +562,157 @@ export function PullRequestDetailPanel({ void refreshFromHost(); }, [forcedRefreshToken, refreshFromHost]); const runAction = useAtomCommand(pullRequestEnvironment.runAction, { reportFailure: false }); - const [actionPending, setActionPending] = useState(false); + // Which action is in flight, not merely that one is: every control here is disabled while any + // of them runs, but only the button that was pressed may say what it is doing. + const [pendingAction, setPendingAction] = useState(null); + const actionPending = pendingAction !== null; + const update = useAtomCommand(pullRequestEnvironment.update, { reportFailure: false }); + // Scoped to the pull request it was typed against, since this one panel shows a different one + // every time it is opened and a half-written title must not follow it there. + const [titleScope, setTitleScope] = useState<{ + readonly pullRequestKey: string; + readonly text: string; + } | null>(null); + const titleDraft = titleScope?.pullRequestKey === pullRequestKey ? titleScope.text : null; + const [titleSaving, setTitleSaving] = useState(false); const newThread = useNewThreadHandler(); + const { environments } = useEnvironments(); + const projects = useProjects(); + // Beside a thread there is nothing to pick: the hand-offs land in that thread's composer, and + // the thread is already on one server's copy of the branch. + const pickableEnvironments = useMemo( + () => + context === "page" + ? resolvePickableEnvironments( + { environmentId, projectId: reference.projectId }, + projects, + environments, + ) + : [], + [context, environmentId, environments, projects, reference.projectId], + ); + // Which server the reader chose, and only for the pull request they chose it on: this one panel + // shows a different pull request every time it is opened, and the choice does not follow. + const [actingScope, setActingScope] = useState<{ + readonly pullRequestKey: string; + readonly environmentId: EnvironmentId; + } | null>(null); + const chosenEnvironmentId = + actingScope?.pullRequestKey === pullRequestKey ? actingScope.environmentId : environmentId; + // Null wherever there is no choice on offer — one server, or a chosen one that has since gone — + // and then the panel's own server and its own checkout are the answer, as they always were. + const acting = + pickableEnvironments.find((entry) => entry.environmentId === chosenEnvironmentId) ?? null; + const actingEnvironmentId = acting?.environmentId ?? environmentId; const prepareThread = usePreparePullRequestThreadAction({ - environmentId, - cwd: detail?.workspaceRoot ?? null, + environmentId: actingEnvironmentId, + cwd: acting?.workspaceRoot ?? detail?.workspaceRoot ?? null, }); - const perform = async (action: PullRequestAction, method?: PullRequestMergeMethod) => { - if (actionPending) return; - setActionPending(true); + const perform = async ( + action: PullRequestAction, + method?: PullRequestMergeMethod, + updateMethod?: PullRequestUpdateMethod, + ) => { + if (pendingAction !== null) return; + setPendingAction(action); const result = await runAction({ environmentId, - input: { ...reference, action, ...(method ? { mergeMethod: method } : {}) }, + input: { + ...reference, + action, + ...(method ? { mergeMethod: method } : {}), + ...(updateMethod ? { updateMethod } : {}), + }, }); - setActionPending(false); + setPendingAction(null); if (result._tag === "Failure") { // The host's own sentence, because it is the only thing that says why. A merge strategy a // branch policy forbids is refused at completion and nowhere earlier — Azure DevOps // publishes no per-strategy availability to hide the control with — so "action failed" // would leave the reader pressing the same button again. const failure = squashAtomCommandFailure(result); + // The hint stands for what was actually asked for: a reader who pressed Update branch is + // told to check their access, not offered the merge commit they already chose. + const hint = + updateMethod === "rebase" + ? UPDATE_BRANCH_REBASE_FAILURE_HINT + : ACTION_FAILURE_HINTS[action]; toastManager.add({ type: "error", title: ACTION_FAILURE_LABELS[action], - description: readableFailure(failure, ACTION_FAILURE_HINTS[action]), + description: readableFailure(failure, hint), }); return; } toastManager.add({ type: "success", title: ACTION_SUCCESS_LABELS[action] }); - refreshDetail(); + // A branch update moves the head commit, which leaves the diff atom pointed at a comparison + // that no longer exists — the same staleness the manual refresh button fixes, so it goes + // through that path rather than a second one. Every other action here only changes metadata; + // a merge does move the branch too, but it also closes the pull request, where the diff is + // no longer what anyone is looking at. + if (pullRequestActionNeedsHostRefresh(action)) { + void refreshFromHost(); + } else { + refreshDetail(); + } onActed?.(); }; + const saveTitle = async (next: string) => { + const title = next.trim(); + if (detail === null || titleSaving) return; + if (title.length === 0 || title === detail.title) { + setTitleScope(null); + return; + } + setTitleSaving(true); + const result = await update({ environmentId, input: { ...reference, title } }); + setTitleSaving(false); + if (result._tag === "Failure") { + // The draft stays open with the words still in it: retyping a title somebody has just + // rewritten is the one thing a failed save must not cost them. + toastManager.add({ + type: "error", + title: "The title could not be saved", + description: readableFailure( + squashAtomCommandFailure(result), + "The host refused the new title.", + ), + }); + return; + } + setTitleScope(null); + refreshDetail(); + }; + type ThreadTask = { prompt: string; reviewComments?: ReadonlyArray; }; + // Beside the thread whose own pull request this is, a task belongs in that thread's composer: + // the branch is already checked out under it, so opening a second thread would only scatter + // the work. + const attachTarget = pullRequestComposerTarget(context, composerDraftTarget); + const handoffLabels = pullRequestHandoffLabels(attachTarget !== null); + + const writeTaskToComposer = (target: ScopedThreadRef | DraftId, task: ThreadTask) => { + const store = useComposerDraftStore.getState(); + const draft = store.getComposerDraft(target); + const key = composerTargetKey(target); + const prompt = handoffPrompt( + { prompt: draft?.prompt ?? "", lastHandoffPrompt: lastHandoffPromptByDraft.get(key) }, + task.prompt, + ); + lastHandoffPromptByDraft.set(key, task.prompt); + store.setPrompt(target, prompt); + store.setReviewComments( + target, + handoffReviewComments(draft?.reviewComments ?? [], task.reviewComments ?? []), + ); + }; + /** * Opens a thread on this project and leaves the task in its composer for the reader to send. * @@ -413,37 +732,32 @@ export function PullRequestDetailPanel({ () => null, )); if (session === null) return null; - const store = useComposerDraftStore.getState(); if (task === null) return session; // The latest press is the ask: it takes over what an earlier hand-off left, prompt and chips // both, rather than stacking a second one under the first. What the reader typed themselves // survives — the composer they are handed is not always a fresh one, and a prompt they have // since edited is theirs rather than the hand-off's. - const draft = store.getComposerDraft(session.draftId); - const existingComments = draft?.reviewComments ?? []; - const prompt = handoffPrompt( - { - prompt: draft?.prompt ?? "", - lastHandoffPrompt: lastHandoffPromptByDraft.get(session.draftId), - }, - task.prompt, - ); - // Remember the hand-off's own contribution, not the merged prompt: only that sentence is - // this session's to take back next time, and the reader's text around it is not. - lastHandoffPromptByDraft.set(session.draftId, task.prompt); - store.setPrompt(session.draftId, prompt); - store.setReviewComments( - session.draftId, - handoffReviewComments(existingComments, task.reviewComments ?? []), - ); + writeTaskToComposer(session.draftId, task); return session; }; /** A question about the change, which needs a thread and nothing else. */ const startAsk = async (kind: string, task: ThreadTask) => { if (!detail || handoff !== null) return; + if (attachTarget !== null) { + writeTaskToComposer(attachTarget, task); + toastManager.add({ + type: "success", + title: "Added to the composer", + description: + task.prompt.length > 0 + ? "The question is in the composer — read it over, then send." + : "The pull request is in the composer — type your question, then send.", + }); + return; + } setHandoff(kind); - const projectRef = scopeProjectRef(environmentId, detail.projectId); + const projectRef = scopeProjectRef(actingEnvironmentId, acting?.projectId ?? detail.projectId); const opened = await openThreadWithTask(projectRef, task); setHandoff(null); if (opened === null) { @@ -478,6 +792,15 @@ export function PullRequestDetailPanel({ mode: "worktree" | "local" = "worktree", ) => { if (!detail || handoff !== null) return; + if (attachTarget !== null && task !== null) { + writeTaskToComposer(attachTarget, task); + toastManager.add({ + type: "success", + title: "Added to the composer", + description: "The task is in the composer — read it over, then send.", + }); + return; + } setHandoff(kind); // The menu closes on the press and takes its "Preparing..." label with it, so this is the // only thing answering for the checkout. It carries no timeout of its own: a loading toast @@ -486,7 +809,9 @@ export function PullRequestDetailPanel({ type: "loading", title: "Preparing the pull request checkout...", }); - const projectRef = scopeProjectRef(environmentId, detail.projectId); + // Wherever the reader chose to act: the thread, the checkout it is pointed at and the composer + // the task lands in are all one server's, and picking another one moves all three. + const projectRef = scopeProjectRef(actingEnvironmentId, acting?.projectId ?? detail.projectId); // The thread is opened before the checkout rather than after it, because the project's setup // script only runs for a checkout that knows which thread it is for — and a worktree with no // dependencies installed is not something anyone can test. @@ -613,20 +938,20 @@ export function PullRequestDetailPanel({ }); }; - /** Lines the reader marked in the diff, asked about rather than commented on. */ - const askAboutSelection = (selection: PullRequestAskSelectionInput) => { + const addSelectionToAgent = (selection: PullRequestAgentSelectionInput) => { if (!detail) return; - void startAsk(`ask:${selection.comment.id}`, { - ...buildAskAboutLinesHandoff({ + void startAsk( + `selection:${selection.comment.id}`, + buildAddSelectionToAgentHandoff({ number: detail.number, title: detail.title, url: detail.url, headBranch: detail.headBranch, baseBranch: detail.baseBranch, comment: selection.comment, - question: selection.question, + request: selection.request, }), - }); + ); }; const startCheckout = (mode: "worktree" | "local") => { @@ -689,6 +1014,12 @@ export function PullRequestDetailPanel({ ? mergeMethod : (allowedMergeMethods[0] ?? "merge"); const conflicting = detail?.state === "open" && detail.mergeability === "conflicting"; + // Only an outright yes arms it. A host that reports nothing has not said the merge is already + // spoken for, and an off switch for something that may not be on says the wrong thing twice. + const autoMergeArmed = detail?.state === "open" && detail.autoMergeEnabled === true; + // Out of date with the base, and still cleanly mergeable — the one pairing an update button + // exists for. Null everywhere else, including hosts that cannot compare at all. + const freshness = detail === null ? null : resolveBaseFreshness(detail); // A host that cannot produce a patch has no Code tab to open. The tabs themselves stay hidden // until the detail arrives, so the loading ghost is the panel's only unfinished UI. const visibleTabs = TABS.filter( @@ -720,12 +1051,46 @@ export function PullRequestDetailPanel({ : allowedMergeMethods.length > 0 ? "merge" : null; + // What the menu's action group holds. Named once so the separators around it are drawn from + // the same answer as its contents, rather than on the assumption that it has any. + const showsDraftToggle = + detail?.state === "open" && + can(detail.isDraft ? "ready" : "draft") && + !(detail.isDraft && primaryAction === "ready"); + const showsAutoMerge = + detail?.state === "open" && + ((autoMergeArmed && can("disable-auto-merge")) || + (!autoMergeArmed && + !detail.isDraft && + !conflicting && + can("enable-auto-merge") && + allowedMergeMethods.length > 0)); + const showsMergeMethods = + detail?.state === "open" && + can("merge") && + !detail.isDraft && + !conflicting && + allowedMergeMethods.length > 1; // The pull request number carries this state in the overview and the right-panel tab mirrors // it. Conflicts keep their own row below: an open pull request remains green there. const statePresentation = detail ? resolvePullRequestState({ state: detail.state, isDraft: detail.isDraft }) : null; const checksSummary = detail ? summarizePullRequestChecks(detail.checks) : null; + const checksState = detail ? pullRequestChecksState(detail.checks) : null; + // Approvals that still stand, and only those. A superseded one is dimmed beside the reviewer + // who gave it, so counting it here would have the header assert in a number what the row next + // to it has just qualified. + // + // Not counted at all from a conversation this page only holds the recent end of: an approval + // older than the window would be missing, and "1" beside a tick is read as the whole answer. + // The Summary tab's row can say it may be short; a bare number cannot, so it stays away. + const approvalCount = + detail && !detail.commentsTruncated + ? latestPullRequestReviewOutcomes(detail.comments, detail.commits).filter( + (entry) => entry.outcome === "approved" && !entry.stale, + ).length + : 0; return (
    @@ -750,21 +1115,31 @@ export function PullRequestDetailPanel({ > {detail && statePresentation ? ( <> - - {detail.repository} - - + + {detail.repository}} + /> + {detail.repository} + + + void readLocalApi()?.shell.openExternal(detail.url)} + onContextMenu={(event) => openNumberContextMenu(event, detail)} + className={cn( + "shrink-0 font-medium underline-offset-2 hover:underline", + statePresentation.toneClassName, + )} + aria-label={`Open pull request #${detail.number} on host`} + > + #{detail.number} + + } + /> + {openOnHostLabel(detail.provider)} + ) : null}
    @@ -780,22 +1155,36 @@ export function PullRequestDetailPanel({ > {detail && statePresentation ? ( <> - - - {detail.title} - + + void readLocalApi()?.shell.openExternal(detail.url)} + onContextMenu={(event) => openNumberContextMenu(event, detail)} + className={cn( + "shrink-0 font-medium underline-offset-2 hover:underline", + statePresentation.toneClassName, + )} + aria-label={`Open pull request #${detail.number} on host`} + > + #{detail.number} + + } + /> + {openOnHostLabel(detail.provider)} + + + + {detail.title} + + } + /> + {detail.title} + {conflicting ? ( ) : checksSummary ? ( - + + {detail && checksState !== null ? ( + + ) : null} {checksSummary} ) : null} @@ -821,8 +1213,14 @@ export function PullRequestDetailPanel({ + } /> } > @@ -840,7 +1238,9 @@ export function PullRequestDetailPanel({ {handoff === "ask" ? "Opening..." : "Ask a question"} - Opens a thread that knows which pull request you mean. + {attachTarget !== null + ? "Adds the pull request to this thread's composer." + : "Opens a thread that knows which pull request you mean."} @@ -855,16 +1255,23 @@ export function PullRequestDetailPanel({ - {handoff === "findings" ? "Preparing..." : "Fix findings in a thread"} + {handoff === "findings" ? "Preparing..." : handoffLabels.fixFindings} + {pickableEnvironments.length > 0 ? ( + setActingScope({ pullRequestKey, environmentId: next })} + disabled={handoff !== null} + /> + ) : null} {detail.state === "open" ? ( <> {/* Only where the button row could not take it: "Ready for review" on a draft is the primary header button, so offering it here as well would show the same action twice. */} - {can(detail.isDraft ? "ready" : "draft") && - !(detail.isDraft && primaryAction === "ready") ? ( + {showsDraftToggle ? ( void perform(detail.isDraft ? "ready" : "draft")} @@ -877,17 +1284,43 @@ export function PullRequestDetailPanel({ {detail.isDraft ? "Ready for review" : "Convert to draft"} ) : null} + {/* The same merge, left with the host to carry out once the things it + waits on are done. It is offered beside the merge rather than instead + of it, because the reader who can wait and the reader who cannot are + the same person on different days — and a conflicting branch is neither, + since nothing the host waits for will clear it. */} + {autoMergeArmed && can("disable-auto-merge") ? ( + void perform("disable-auto-merge")} + > + + Disable auto-merge + + ) : !autoMergeArmed && + !detail.isDraft && + !conflicting && + can("enable-auto-merge") && + allowedMergeMethods.length > 0 ? ( + setConfirmAction("enable-auto-merge")} + > + + Enable auto-merge + + ) : null} {/* A preference for the merge action rather than a second action, so it is a radio group here instead of a chevron welded to the Merge pill. Hidden while conflicting: every method would fail. */} {/* Only where merging is on offer at all: a strategy to merge with is not a choice for someone who may not merge. */} - {can("merge") && - !detail.isDraft && - !conflicting && - allowedMergeMethods.length > 1 ? ( + {showsMergeMethods ? ( <> - + {/* Only below the draft control. A host with no draft of its own, or + a draft whose control is already the header button, would leave + this against the separator that opened the group. */} + {showsDraftToggle ? : null} @@ -907,12 +1340,18 @@ export function PullRequestDetailPanel({ ) : null} - + {pullRequestActionMenuHasGroup( + showsDraftToggle, + showsAutoMerge, + showsMergeMethods, + ) ? ( + + ) : null} ) : null} void readLocalApi()?.shell.openExternal(detail.url)}> - {OPEN_ON_HOST_LABELS[detail.provider] ?? "Open on host"} + {openOnHostLabel(detail.provider)} void writeTextToClipboard(detail.url)}> @@ -922,7 +1361,7 @@ export function PullRequestDetailPanel({ {conflicting && primaryAction !== "resolve" ? ( - {handoff === "conflicts" ? "Preparing..." : "Resolve conflicts in a thread"} + {handoff === "conflicts" ? "Preparing..." : handoffLabels.resolveConflicts} ) : null} {detail.state === "open" && can("close") ? ( @@ -990,9 +1429,37 @@ export function PullRequestDetailPanel({ + {pickableEnvironments.length > 0 ? ( + setActingScope({ pullRequestKey, environmentId: next })} + disabled={handoff !== null} + /> + ) : null} ) : null} + {/* Said where the Merge button is, because it is the answer to why nobody has + pressed it: the merge is already asked for, and the host is holding it. */} + {autoMergeArmed ? ( + + + + Auto-merge + + } + /> + + The host will merge this on its own once its requirements are met + + + ) : null} {primaryAction === "ready" ? ( ) : null} @@ -1030,7 +1497,16 @@ export function PullRequestDetailPanel({ {/* The condensed chrome's second row: the tabs that the closing fold takes with it, and compact copies of the branch pair and diff stat so they stay in sight while the full rows are folded away. Same zero-track mechanism as the fold, inverted. */} -
    +
    ))} - - {detail.baseBranch} + + + }> + {detail.baseBranch} + + {`${detail.baseBranch} ← ${detail.headBranch}`} + + {freshness ? ( + void perform("update-branch", undefined, method)} + iconClassName="size-3" + /> + ) : null} - {detail.headBranch} + + }> + {detail.headBranch} + + {`${detail.baseBranch} ← ${detail.headBranch}`} + @@ -1092,10 +1584,13 @@ export function PullRequestDetailPanel({
    {detail ? (
    -

    {detail.title}

    + {titleDraft === null ? ( +
    +

    + {detail.title} +

    + {canEditPullRequestChangeRequest(detail) ? ( + + ) : null} +
    + ) : ( + // A title is one line of text, not markdown, so it takes an input rather than + // the editor the description and the remarks share. +
    + + setTitleScope({ pullRequestKey, text: event.target.value }) + } + onKeyDown={(event) => { + if (event.key === "Enter") { + event.preventDefault(); + void saveTitle(titleDraft); + } else if (event.key === "Escape") { + event.preventDefault(); + setTitleScope(null); + } + }} + /> +
    + + +
    +
    + )} updated {formatRelativeTimeLabel(detail.updatedAt)}
    - - {detail.baseBranch} - + + + {detail.baseBranch} + + } + /> + {detail.baseBranch} + + {freshness ? ( + void perform("update-branch", undefined, method)} + /> + ) : null} - + + {detail.headBranch} + + + + + {`${isBranchCopied ? "Copied" : "Copy pull request branch"}: ${detail.headBranch}`} + + @@ -1185,7 +1760,7 @@ export function PullRequestDetailPanel({ disabled={handoff !== null} onClick={startResolveConflicts} > - {handoff === "conflicts" ? "Preparing..." : "Resolve in a new thread"} + {handoff === "conflicts" ? "Preparing..." : handoffLabels.resolve}
    @@ -1193,7 +1768,7 @@ export function PullRequestDetailPanel({ {detail ? (
    @@ -1371,13 +1974,14 @@ export function PullRequestDetailPanel({
    }> - {confirmAction === "merge" ? "Merge pull request?" : "Close pull request?"} + {confirmAction === "merge" + ? "Merge pull request?" + : confirmAction === "enable-auto-merge" + ? "Enable auto-merge?" + : "Close pull request?"} {confirmAction === "merge" ? `This merges #${reference.number} using ${selectedMergeMethod}.` - : `This closes #${reference.number} without merging it.`} + : confirmAction === "enable-auto-merge" + ? // The host merges this as soon as it considers the pull request ready, which + // may be immediately — there is no telling from here whether anything is + // still outstanding. + `This merges #${reference.number} using ${selectedMergeMethod} as soon as the host considers it ready, which may be immediately.` + : `This closes #${reference.number} without merging it.`} @@ -1416,10 +2029,16 @@ export function PullRequestDetailPanel({ const action = confirmAction; setConfirmAction(null); if (action === "merge") void perform("merge", selectedMergeMethod); + if (action === "enable-auto-merge") + void perform("enable-auto-merge", selectedMergeMethod); if (action === "close") void perform("close"); }} > - {confirmAction === "merge" ? "Merge" : "Close"} + {confirmAction === "merge" + ? "Merge" + : confirmAction === "enable-auto-merge" + ? "Enable auto-merge" + : "Close"} diff --git a/apps/web/src/components/pullRequest/PullRequestListFilters.test.tsx b/apps/web/src/components/pullRequest/PullRequestListFilters.test.tsx index 545e3066f81d..f1c3013167f7 100644 --- a/apps/web/src/components/pullRequest/PullRequestListFilters.test.tsx +++ b/apps/web/src/components/pullRequest/PullRequestListFilters.test.tsx @@ -1,9 +1,9 @@ -import type { ProjectId } from "@t3tools/contracts"; +import type { EnvironmentId, ProjectId } from "@t3tools/contracts"; import { CircleIcon } from "lucide-react"; import { Children, isValidElement, type ReactElement, type ReactNode } from "react"; import { describe, expect, it, vi } from "vite-plus/test"; -import { PullRequestFiltersMenu } from "./PullRequestListFilters"; +import { PullRequestFiltersMenu, pullRequestProjectKey } from "./PullRequestListFilters"; function findValueChange( node: ReactNode, @@ -53,12 +53,17 @@ function menu(overrides: Partial[0]>) involvement: "all", involvementOptions: [{ value: "all", label: "All", Icon: CircleIcon }], onInvolvement: () => undefined, + filters: {}, + onFilters: () => undefined, host: undefined, hostOptions: [], onHost: () => undefined, - environmentId: null, + server: undefined, + serverOptions: [], + onServer: () => undefined, projects: [], projectId: undefined, + projectEnvironmentId: undefined, unavailable: new Map(), onProject: () => undefined, ...overrides, @@ -79,21 +84,98 @@ describe("pull request filters menu", () => { expect(onState).toHaveBeenCalledWith("closed"); }); + it("names the chosen narrowing and leaves the others alone", () => { + const onFilters = vi.fn(); + const group = findValueChange( + findLabeledGroup(menu({ filters: { review: "approved" }, onFilters }), "Draft"), + ); + expect(group).toBeDefined(); + + group?.props.onValueChange("hide"); + expect(onFilters).toHaveBeenCalledWith({ review: "approved", draft: "hide" }); + }); + + it("drops a narrowing chosen back to all rather than sending it as undefined", () => { + const onFilters = vi.fn(); + const group = findValueChange( + findLabeledGroup( + menu({ filters: { review: "none", checks: "failing" }, onFilters }), + "Review", + ), + ); + expect(group).toBeDefined(); + + group?.props.onValueChange("all"); + expect(onFilters).toHaveBeenCalledWith({ checks: "failing" }); + }); + it("does not emit a change when the selected project is chosen again", () => { const projectId = "project-1" as ProjectId; + const environmentId = "env-1" as EnvironmentId; const onProject = vi.fn(); const view = menu({ - projects: [{ id: projectId, title: "T3 Code", workspaceRoot: "/work/t3code" }], + projects: [ + { + id: projectId, + environmentId, + title: "T3 Code", + workspaceRoot: "/work/t3code", + }, + ], projectId, + projectEnvironmentId: environmentId, onProject, }); const radioGroup = findValueChange(view); expect(radioGroup).toBeDefined(); - radioGroup?.props.onValueChange(projectId); + radioGroup?.props.onValueChange(pullRequestProjectKey({ id: projectId, environmentId })); expect(onProject).not.toHaveBeenCalled(); radioGroup?.props.onValueChange("all"); - expect(onProject).toHaveBeenCalledWith(undefined); + expect(onProject).toHaveBeenCalledWith(undefined, undefined); + }); + + it("passes the environment along so a duplicate project id on another server is told apart", () => { + const projectId = "project-1" as ProjectId; + const onProject = vi.fn(); + const view = menu({ + projects: [ + { + id: projectId, + environmentId: "env-1" as EnvironmentId, + title: "T3 Code · one", + workspaceRoot: "/work/t3code-1", + }, + { + id: projectId, + environmentId: "env-2" as EnvironmentId, + title: "T3 Code · two", + workspaceRoot: "/work/t3code-2", + }, + ], + onProject, + }); + const radioGroup = findValueChange(view); + expect(radioGroup).toBeDefined(); + + radioGroup?.props.onValueChange( + pullRequestProjectKey({ id: projectId, environmentId: "env-2" as EnvironmentId }), + ); + expect(onProject).toHaveBeenCalledWith(projectId, "env-2"); + }); + + it("does not collide when environment and project ids contain spaces", () => { + expect( + pullRequestProjectKey({ + environmentId: "a b" as EnvironmentId, + id: "c" as ProjectId, + }), + ).not.toBe( + pullRequestProjectKey({ + environmentId: "a" as EnvironmentId, + id: "b c" as ProjectId, + }), + ); }); }); diff --git a/apps/web/src/components/pullRequest/PullRequestListFilters.tsx b/apps/web/src/components/pullRequest/PullRequestListFilters.tsx index 332772e405e1..d4c8f147ff30 100644 --- a/apps/web/src/components/pullRequest/PullRequestListFilters.tsx +++ b/apps/web/src/components/pullRequest/PullRequestListFilters.tsx @@ -2,15 +2,29 @@ import type { EnvironmentId, ProjectId, PullRequestInvolvement, + PullRequestListFilters, PullRequestListState, SourceControlProviderKind, } from "@t3tools/contracts"; -import { FolderGit2Icon, LayersIcon, ListFilterIcon, LoaderIcon, SearchIcon } from "lucide-react"; +import { + CircleCheckIcon, + CircleDashedIcon, + CircleSlashIcon, + CircleXIcon, + EyeOffIcon, + FolderGit2Icon, + GitPullRequestDraftIcon, + LayersIcon, + ListFilterIcon, + LoaderIcon, + SearchIcon, +} from "lucide-react"; import type { ElementType } from "react"; import { cn } from "~/lib/utils"; import { getSourceControlPresentationForKind } from "~/sourceControlPresentation"; import { ProjectFavicon } from "../ProjectFavicon"; +import { InputGroup, InputGroupAddon, InputGroupInput } from "../ui/input-group"; import { Menu, @@ -67,29 +81,18 @@ export function PullRequestSearchInput({ onChange: (value: string) => void; }) { return ( -
    - {busy ? ( - - ) : ( - - )} - + + {busy ? : } + + onChange(event.currentTarget.value)} - placeholder="Search pull requests" + placeholder="Search pull requests, or label:bug" aria-label="Search pull requests" - // Tracks the shared input's height at both widths, so it stays level with the icon - // button beside it rather than towering over it on wide screens. - className="h-9 w-full rounded-lg border border-input bg-background pr-3 pl-9 text-sm outline-none placeholder:text-muted-foreground/72 focus-visible:border-ring focus-visible:ring-[3px] focus-visible:ring-ring/24 sm:h-8" /> -
    + ); } @@ -102,6 +105,38 @@ export function PullRequestSearchInput({ const ALL_PROJECTS_VALUE = "all"; /** MenuRadioGroup wants a string, so "every host" wears the one value no host can be. */ const ALL_HOSTS_VALUE = ""; +/** The same trick for the servers, which are named by an id no empty string can collide with. */ +const ALL_SERVERS_VALUE = ""; +/** The unset value of each narrowing group, which no filter of theirs is named after. */ +const UNFILTERED_VALUE = "all"; +/** + * A project's own radio value, carrying the server along with the id: the id alone is only + * unique within its own server, so two rows sharing one would otherwise both read as checked. + */ +export const pullRequestProjectKey = (project: { + readonly id: ProjectId; + readonly environmentId: EnvironmentId; +}) => JSON.stringify([project.environmentId, project.id]); + +const DRAFT_OPTIONS = [ + { value: UNFILTERED_VALUE, label: "All", Icon: LayersIcon }, + { value: "only", label: "Drafts only", Icon: GitPullRequestDraftIcon }, + { value: "hide", label: "Hide drafts", Icon: EyeOffIcon }, +] as const satisfies ReadonlyArray>; + +const REVIEW_OPTIONS = [ + { value: UNFILTERED_VALUE, label: "All", Icon: LayersIcon }, + { value: "approved", label: "Approved", Icon: CircleCheckIcon }, + { value: "changes-requested", label: "Changes requested", Icon: CircleXIcon }, + { value: "review-required", label: "Review required", Icon: CircleDashedIcon }, + { value: "none", label: "No reviews", Icon: CircleSlashIcon }, +] as const satisfies ReadonlyArray>; + +const CHECKS_OPTIONS = [ + { value: UNFILTERED_VALUE, label: "All", Icon: LayersIcon }, + { value: "passing", label: "Passing", Icon: CircleCheckIcon }, + { value: "failing", label: "Failing", Icon: CircleXIcon }, +] as const satisfies ReadonlyArray>; function PullRequestFilterRadioGroup({ label, @@ -122,21 +157,32 @@ function PullRequestFilterRadioGroup({ }} > {label} - {options.map((option) => ( - - - - {option.label} - - - ))} + {options.map((option) => { + // A host the server has already said it cannot read is not a choice here: offering + // it would answer the press by replacing a working list with that failure. + const item = ( + + + + {option.label} + + + ); + if (!option.unavailable) return item; + return ( + + + + {option.unavailable} + + + ); + })} ); } @@ -148,12 +194,17 @@ export function PullRequestFiltersMenu({ involvement, involvementOptions, onInvolvement, + filters, + onFilters, host, hostOptions, onHost, - environmentId, + server, + serverOptions, + onServer, projects, projectId, + projectEnvironmentId, unavailable, onProject, }: { @@ -163,6 +214,9 @@ export function PullRequestFiltersMenu({ involvement: PullRequestInvolvement; involvementOptions: ReadonlyArray>; onInvolvement: (involvement: PullRequestInvolvement) => void; + /** The narrowings beyond state and involvement; an absent field is that group unfiltered. */ + filters: PullRequestListFilters; + onFilters: (filters: PullRequestListFilters) => void; host: string | undefined; /** * Includes the "all hosts" entry, whose value is the empty string. With fewer than two real @@ -170,24 +224,52 @@ export function PullRequestFiltersMenu({ */ hostOptions: ReadonlyArray>; onHost: (host: string | undefined) => void; - /** Where the projects' own favicons are read from; null before the environment is known. */ - environmentId: EnvironmentId | null; + server: EnvironmentId | undefined; + /** + * Includes the "all servers" entry, whose value is the empty string. With one server there is + * nothing to switch between, so the whole group stays out of the menu. + */ + serverOptions: ReadonlyArray>; + onServer: (server: EnvironmentId | undefined) => void; + /** The projects of every connected environment, each carrying the one its favicon is read from. */ projects: ReadonlyArray<{ readonly id: ProjectId; + readonly environmentId: EnvironmentId; readonly title: string; readonly workspaceRoot: string; }>; projectId: ProjectId | undefined; + /** + * The server the selected project belongs to. A project id is only unique within its own + * server, so without this two rows sharing an id would both read as checked here. + */ + projectEnvironmentId: EnvironmentId | undefined; /** * Projects whose repository could not be read this time round. They are named here, where * the reader is already choosing between projects, rather than as a count above the list * that says something is missing without saying which. */ - unavailable: ReadonlyMap; - onProject: (projectId: ProjectId | undefined) => void; + unavailable: ReadonlyMap; + /** The environment comes with the project id, since picking a row picks a specific server's copy of it. */ + onProject: (projectId: ProjectId | undefined, environmentId: EnvironmentId | undefined) => void; }) { const filtered = - state !== "open" || involvement !== "all" || host !== undefined || projectId !== undefined; + state !== "open" || + involvement !== "all" || + host !== undefined || + server !== undefined || + projectId !== undefined || + Object.keys(filters).length > 0; + /** + * Rebuilt rather than spread so an unfiltered group leaves the record instead of lingering in + * it as an explicit `undefined`, which the listing input does not accept. + */ + const withFilter = (key: keyof PullRequestListFilters, value: string): PullRequestListFilters => + Object.fromEntries( + Object.entries({ ...filters, [key]: value === UNFILTERED_VALUE ? undefined : value }).filter( + ([, held]) => held !== undefined, + ), + ) as PullRequestListFilters; return ( @@ -227,6 +309,27 @@ export function PullRequestFiltersMenu({ options={involvementOptions} onChange={onInvolvement} /> + + onFilters(withFilter("draft", next))} + /> + + onFilters(withFilter("review", next))} + /> + + onFilters(withFilter("checks", next))} + /> {hostOptions.length > 2 ? ( <> @@ -238,12 +341,40 @@ export function PullRequestFiltersMenu({ /> ) : null} + {serverOptions.length > 2 ? ( + <> + + + onServer(next === ALL_SERVERS_VALUE ? undefined : (next as EnvironmentId)) + } + /> + + ) : null} { - const nextProjectId = next === ALL_PROJECTS_VALUE ? undefined : (next as ProjectId); - if (nextProjectId !== projectId) onProject(nextProjectId); + if (next === ALL_PROJECTS_VALUE) { + if (projectId !== undefined) onProject(undefined, undefined); + return; + } + // The value carries both halves, since the id alone cannot tell two servers' rows + // apart once they share one. + const project = projects.find((candidate) => pullRequestProjectKey(candidate) === next); + if ( + project !== undefined && + (project.id !== projectId || project.environmentId !== projectEnvironmentId) + ) { + onProject(project.id, project.environmentId); + } }} > Project @@ -257,28 +388,26 @@ export function PullRequestFiltersMenu({ as a broken menu rather than as a workspace with three unreadable repositories. */} {projects .toSorted( - (left, right) => Number(unavailable.has(left.id)) - Number(unavailable.has(right.id)), + (left, right) => + Number(unavailable.has(pullRequestProjectKey(left))) - + Number(unavailable.has(pullRequestProjectKey(right))), ) .map((project) => { - const reason = unavailable.get(project.id); - return ( + const reason = unavailable.get(pullRequestProjectKey(project)); + const item = ( - {environmentId === null ? ( - - ) : ( - - )} + {project.title} {reason === undefined ? null : ( @@ -288,6 +417,15 @@ export function PullRequestFiltersMenu({ ); + if (reason === undefined) return item; + return ( + + + + {reason} + + + ); })} diff --git a/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx b/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx new file mode 100644 index 000000000000..f0145c059c0d --- /dev/null +++ b/apps/web/src/components/pullRequest/PullRequestMarkdownEditor.tsx @@ -0,0 +1,113 @@ +import { useState } from "react"; + +import { cn } from "~/lib/utils"; + +import { Button } from "../ui/button"; +import { Textarea } from "../ui/textarea"; +import { PullRequestMarkdown } from "./PullRequestMarkdown"; + +/** + * The box a body is rewritten in — a description, or a remark already posted. It owns the draft + * and nothing else: the caller sends the request and says whether it is still in flight, so the + * same box serves every mutation without knowing which one it is. + * + * Preview renders through the same component the saved body will be read through, which is the + * only way to see what a host's markdown will actually become before it is sent. + */ +export function PullRequestMarkdownEditor({ + value, + cwd, + placeholder, + label, + saving, + allowEmpty = false, + className, + onSave, + onCancel, +}: { + readonly value: string; + readonly cwd: string; + readonly placeholder?: string | undefined; + readonly label: string; + readonly saving: boolean; + /** A description may be cleared, which is how one is removed; a remark may not be emptied. */ + readonly allowEmpty?: boolean; + readonly className?: string | undefined; + readonly onSave: (next: string) => void; + readonly onCancel: () => void; +}) { + const [draft, setDraft] = useState(value); + const [preview, setPreview] = useState(false); + // The words this draft started from. React keeps a component instance wherever the same + // position and key come round again, so an editor opened on one remark can be handed another's + // words without being rebuilt — and saving would then write the first remark's text onto the + // second. Different words mean a different subject, and the draft starts again from them. + const [seed, setSeed] = useState(value); + if (seed !== value) { + setSeed(value); + setDraft(value); + } + const empty = draft.trim().length === 0; + + return ( +
    { + if (event.key !== "Escape" || saving) return; + event.preventDefault(); + onCancel(); + }} + > +
    + + +
    + {preview ? ( +
    + {empty ? ( +

    Nothing to preview.

    + ) : ( + + )} +
    + ) : ( +