-
Notifications
You must be signed in to change notification settings - Fork 1
Refactor branch divergence UX: quiet markers + undoable switches #95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 13 commits
Commits
Show all changes
22 commits
Select commit
Hold shift + click to select a range
11837db
feat: redesign branch divergence as a quiet branch-control signal
claude 2df18f6
refactor: enforce the gh-* palette; remove stray stock colors
claude d8b6776
feat: unify empty/loading states behind x-empty-state + diff skeleton
claude f34ad11
feat: theme the Flux layer — toast home + on-brand surface, no native…
claude 447e8c1
feat: sharpen the diff gutter and tidy the file-header actions
claude e94f228
feat: sharpen the sidebar and collapse the context tree
claude c096c72
feat: chrome consistency + motion polish
claude d8b7677
feat: unify diff expand controls behind one "Show" affordance
claude 603e849
feat: unfold the comment form on open instead of popping in
claude 55f4c68
refactor: trim expand-control props and dedupe the expander button co…
claude 83f1b90
fix: recompute divergence after undoing a branch switch
claude 34c2054
fix: address PR review feedback on expanders and motion
claude 5a2e21c
test: stabilize browser tests against the redesigned UI
claude 160afb3
fix: de-flake draft re-open test and rework the stale cancel test
claude 483b6cc
fix: restore keyboard focus after expanding a diff gap
claude 0ee6bd1
test: harden gap-refocus browser test against slow CI runners
claude 42ee9f1
test: stabilize Csrf419 reload test under parallel contention
claude e09fa2e
Fix stuck expand spinner when expandGap returns early
claude 7a28ab0
ci: raise benchmark-perf absolute floor to 15ms
claude 7eac844
fix: scope expand spinner clear per-file; harden path collapse + draf…
claude 2f7d4c6
Fix split diff comment anchoring
fgilio 4732d2d
Strengthen split diff anchoring tests
fgilio File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| {{-- | ||
| Content-shaped placeholder for a diff file while its hunks load — a few | ||
| line-number + content bars in the diff's own rhythm, so the layout doesn't | ||
| jump when the real diff arrives. `animate-pulse` is collapsed by the global | ||
| reduce-motion rule. Decorative, so aria-hidden. | ||
| --}} | ||
| @props(['rows' => 7]) | ||
|
|
||
| @php | ||
| // Varied bar widths read as code rather than a flat block. | ||
| $widths = [86, 58, 72, 44, 90, 64, 50, 78, 38, 68]; | ||
| @endphp | ||
|
|
||
| <div class="px-4 py-3 motion-safe:animate-pulse" aria-hidden="true"> | ||
| @for($i = 0; $i < $rows; $i++) | ||
| <div class="flex items-center gap-3 py-1"> | ||
| <div class="h-3 w-6 rounded-sm bg-gh-border/60 shrink-0"></div> | ||
| <div class="h-3 rounded-sm bg-gh-border/40" style="width: {{ $widths[$i % count($widths)] }}%"></div> | ||
| </div> | ||
| @endfor | ||
| </div> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| {{-- | ||
| A clickable target inside <x-diff.expand-control>. Centralizes the expand | ||
| button's wire contract so each call site names its Livewire `action` exactly | ||
| once — it drives wire:click, wire:target, and the markDiffActionStart timing | ||
| mark, and flips the shell's shared `loading` Alpine flag on click. The | ||
| gh-link affordance styling is the base; callers merge per-button classes | ||
| (chip padding, tabular-nums) via the class attribute. | ||
| --}} | ||
| @props([ | ||
| 'action', | ||
| 'args' => '', | ||
| ]) | ||
|
|
||
| <button | ||
| wire:click="{{ $action }}({{ $args }})" | ||
| wire:loading.attr="disabled" | ||
| wire:target="{{ $action }}" | ||
| @click="loading = true; markDiffActionStart('{{ $action }}')" | ||
| {{ $attributes->class('text-gh-link hover:text-gh-text transition-colors disabled:opacity-50') }} | ||
| >{{ $slot }}</button> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| {{-- | ||
| Unified chrome for every collapsed-content affordance in a diff: the | ||
| "show full file" master expander, per-gap tiered expanders, the trailing | ||
| gap, and the (non-expandable) section-context label. | ||
|
|
||
| Coherence contract: the verb ("Show") is rendered here, once, as a muted | ||
| lead-in — never inside a button. Everything blue (gh-link) is clickable and | ||
| lives in the slot: "full file", "8 hidden lines", the "15 · 50 · 100" chips. | ||
| So every expander reads `↕ Show <blue target>` with identical styling. | ||
|
|
||
| Expandable bands carry the `expand-all` (↕) icon, a dashed top/bottom rule | ||
| (dashed = collapsed content lives here), and own a shared `loading` Alpine | ||
| flag so any button in the slot can swap the row to a spinner with | ||
| `@click="loading = true"`. The section-context label passes `:icon="false"` | ||
| (and `align="start"`) — no affordance, no verb, no rule, just the context. | ||
| --}} | ||
| @props([ | ||
| 'icon' => 'expand-all', | ||
| 'align' => 'center', | ||
| ]) | ||
|
|
||
| @php | ||
| $hasIcon = $icon !== false && $icon !== null; | ||
| @endphp | ||
|
|
||
| <div | ||
| {{ $attributes->class([ | ||
| 'diff-fullspan bg-gh-hunk-bg px-4 py-1.5 text-xs font-mono flex items-center gap-2', | ||
| 'border-y border-dashed border-gh-border/20' => $hasIcon, | ||
| 'justify-center select-none' => $align === 'center', | ||
| 'justify-start' => $align === 'start', | ||
| ]) }} | ||
| @if($hasIcon) x-data="{ loading: false }" @endif | ||
| > | ||
| @if($hasIcon) | ||
| <flux:icon :icon="$icon" variant="outline" class="!size-3.5 shrink-0 text-gh-muted/50" x-show="!loading" /> | ||
| <span x-show="!loading" class="inline-flex min-w-0 items-center gap-2"> | ||
| <span class="text-gh-muted/60">Show</span> | ||
| {{ $slot }} | ||
| </span> | ||
| <flux:icon icon="arrow-path" variant="outline" class="!size-3.5 shrink-0 animate-spin text-gh-muted" x-show="loading" x-cloak /> | ||
| <span x-show="loading" x-cloak class="text-gh-muted">Loading…</span> | ||
| @else | ||
| {{ $slot }} | ||
| @endif | ||
| </div> | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| {{-- | ||
| Branch-divergence marker for the header branch control. | ||
|
|
||
| Renders a small dot on the branch pill when the repo's checked-out HEAD has | ||
| drifted from the review target, with a popover offering the choice. Replaces | ||
| the old full-width warning band so the canvas stays calm — the signal lives | ||
| where its cause does. Only shows for the quiet, recoverable states | ||
| (Diverged / Detached); MissingTarget is a blocking bar (<x-divergence.missing-bar>). | ||
|
|
||
| Buttons call ReviewPage actions directly (this is rendered inside its DOM). | ||
| --}} | ||
| @props(['state', 'context' => []]) | ||
|
|
||
| @php | ||
| $isDiverged = $state === \App\Enums\DivergenceState::Diverged; | ||
| $isDetached = $state === \App\Enums\DivergenceState::Detached; | ||
| @endphp | ||
|
|
||
| @if($isDiverged || $isDetached) | ||
| @php | ||
| $target = $context['target'] ?? ''; | ||
| $currentBranch = $context['currentBranch'] ?? null; | ||
| $shortSha = $context['shortSha'] ?? ''; | ||
| $commentCount = (int) ($context['commentCount'] ?? 0); | ||
|
|
||
| // Literal class strings so Tailwind's source scan keeps them in the build. | ||
| $dotClasses = $isDiverged ? 'bg-gh-attention ring-gh-attention/30' : 'bg-gh-link ring-gh-link/30'; | ||
| $testid = $isDiverged ? 'divergence-banner-diverged' : 'divergence-banner-detached'; | ||
| @endphp | ||
|
|
||
| <div x-data="{ open: false }" class="relative inline-flex -ml-0.5" data-testid="{{ $testid }}"> | ||
| <button | ||
| type="button" | ||
| @click="open = ! open" | ||
| @keydown.escape.window="open = false" | ||
| :aria-expanded="open" | ||
| aria-label="{{ $isDiverged ? 'Checkout moved off the review branch — show options' : 'Repo detached from the review branch — show options' }}" | ||
| class="inline-flex items-center justify-center size-6 rounded-md hover:bg-gh-border/40 transition-colors" | ||
| > | ||
| <span class="relative inline-flex h-2 w-2 rounded-full ring-2 {{ $dotClasses }}"></span> | ||
| </button> | ||
|
|
||
| <div | ||
| x-show="open" | ||
| x-cloak | ||
| @click.outside="open = false" | ||
| x-transition:enter="transition ease-out duration-150" | ||
| x-transition:enter-start="opacity-0 -translate-y-1 scale-95" | ||
| x-transition:enter-end="opacity-100 translate-y-0 scale-100" | ||
| x-transition:leave="transition ease-out duration-100" | ||
| x-transition:leave-start="opacity-100 translate-y-0 scale-100" | ||
| x-transition:leave-end="opacity-0 -translate-y-1 scale-95" | ||
| role="dialog" | ||
| class="absolute left-0 top-full mt-2 z-50 w-[320px] origin-top-left rounded-xl border border-gh-border bg-gh-bg shadow-xl shadow-black/10 overflow-hidden" | ||
| > | ||
| <div class="px-4 pt-3.5 pb-3 space-y-2.5"> | ||
| <div class="flex items-center gap-2"> | ||
| <span class="inline-flex h-2 w-2 rounded-full {{ $dotClasses }}"></span> | ||
| <span class="font-display font-semibold tracking-brutal">{{ $isDiverged ? 'Your checkout moved' : 'Repo is detached' }}</span> | ||
| </div> | ||
|
|
||
| @if($isDiverged) | ||
| <p class="text-xs text-gh-muted leading-relaxed"> | ||
| Repo is on <span class="font-mono text-gh-text">{{ $currentBranch }}</span>. | ||
| RFA is still showing your review of <span class="font-mono text-gh-text">{{ $target }}</span>. | ||
| </p> | ||
| @if($commentCount > 0) | ||
| <p class="text-xs text-gh-muted"> | ||
| <span class="text-gh-text font-medium tabular-nums">{{ $commentCount }}</span> | ||
| {{ \Illuminate\Support\Str::plural('comment', $commentCount) }} on <span class="font-mono">{{ $target }}</span> | ||
| </p> | ||
| @endif | ||
| @else | ||
| <p class="text-xs text-gh-muted leading-relaxed"> | ||
| Repo is detached at <span class="font-mono text-gh-text">{{ $shortSha }}</span>. | ||
| RFA is still showing your review of <span class="font-mono text-gh-text">{{ $target }}</span>. | ||
| </p> | ||
| @endif | ||
| </div> | ||
|
|
||
| <div class="flex items-center justify-end gap-1 px-3 py-2.5 border-t border-gh-border bg-gh-surface/50"> | ||
| @if($isDiverged) | ||
| <button type="button" wire:click="keepReviewing" @click="open = false" class="text-xs font-medium text-gh-muted hover:text-gh-text px-2.5 py-1.5 rounded-md transition-colors">Keep reviewing</button> | ||
| <button type="button" wire:click="switchReviewToHead" @click="open = false" class="text-xs font-medium font-display rounded-md px-3 py-1.5 bg-gh-accent text-gh-bg hover:opacity-90 transition-opacity">Switch review here</button> | ||
| @else | ||
| <button type="button" wire:click="dismissDetachedBanner" @click="open = false" class="text-xs font-medium text-gh-muted hover:text-gh-text px-2.5 py-1.5 rounded-md transition-colors">Dismiss</button> | ||
| @endif | ||
| </div> | ||
| </div> | ||
| </div> | ||
| @endif |
38 changes: 38 additions & 0 deletions
38
resources/views/components/divergence/missing-bar.blade.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| {{-- | ||
| Branch-divergence "missing target" bar. | ||
|
|
||
| The one genuinely blocking divergence state: the branch being reviewed no | ||
| longer exists (deleted / renamed / mid-rebase). Unlike Diverged/Detached | ||
| (quiet header marker), this stays a full-width bar — but on-brand (house | ||
| pattern from update-banner, gh-* tokens) rather than a stock-Flux callout, | ||
| and now dismissible. Buttons call ReviewPage actions directly. | ||
| --}} | ||
| @props(['state', 'context' => []]) | ||
|
|
||
| @if($state === \App\Enums\DivergenceState::MissingTarget) | ||
| @php | ||
| $target = $context['target'] ?? ''; | ||
| $currentBranch = $context['currentBranch'] ?? null; | ||
| @endphp | ||
|
|
||
| <div class="bg-gh-surface border-b border-gh-border px-5 py-2.5" role="alert" aria-live="assertive" data-testid="divergence-banner-missing"> | ||
| <div class="flex items-center gap-3"> | ||
| <flux:icon icon="exclamation-triangle" variant="outline" class="!size-4 shrink-0 text-gh-red" /> | ||
| <div class="min-w-0 flex-1"> | ||
| <p class="font-display font-semibold tracking-brutal text-sm">Review target <span class="font-mono">{{ $target }}</span> no longer exists</p> | ||
| <p class="text-xs text-gh-muted"> | ||
| The branch you were reviewing is gone — deleted, renamed, or mid-rebase. | ||
| @if($currentBranch) | ||
| Repo is now on <span class="font-mono text-gh-text">{{ $currentBranch }}</span>. | ||
| @endif | ||
| </p> | ||
| </div> | ||
| <div class="flex items-center gap-1 shrink-0"> | ||
| <button type="button" wire:click="dismissMissingTarget" class="text-xs font-medium text-gh-muted hover:text-gh-text px-2.5 py-1.5 rounded-md transition-colors">Dismiss</button> | ||
| @if($currentBranch) | ||
| <button type="button" wire:click="switchReviewToHead" class="text-xs font-medium font-display rounded-md px-3 py-1.5 bg-gh-accent text-gh-bg hover:opacity-90 transition-opacity">Switch review here</button> | ||
| @endif | ||
| </div> | ||
| </div> | ||
| </div> | ||
| @endif |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.