-
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 15 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
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
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,28 @@ | ||
| {{-- | ||
| 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. | ||
|
|
||
| `gapKey` (the hunk index, set by gap expanders) tags the button with | ||
| data-expand-gap and arms keyboard-only focus restoration: when the | ||
| post-expand re-render replaces this node, focus returns to the expander that | ||
| now sits at the same gap instead of dropping to <body>. Mouse clicks don't | ||
| arm it, and the master "full file" expander leaves it null (no gap remains). | ||
| --}} | ||
| @props([ | ||
| 'action', | ||
| 'args' => '', | ||
| 'gapKey' => null, | ||
| ]) | ||
|
|
||
| <button | ||
| wire:click="{{ $action }}({{ $args }})" | ||
| wire:loading.attr="disabled" | ||
| wire:target="{{ $action }}" | ||
| @if($gapKey !== null) data-expand-gap="{{ $gapKey }}" @endif | ||
| @click="loading = true; markDiffActionStart('{{ $action }}'); armExpandRefocus($event, @js($gapKey))" | ||
| {{ $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> | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| @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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When a gap expander is clicked, this sets the parent
loadingflag immediately, but it only gets cleared by a DOM replacement or the completion event. If the underlying diff changes between render and click soexpandGap()hits one of its early returns (for example the full-context reload has no hunks), no completion event is dispatched and the keyed row does not change, leaving the expander stuck on “Loading…” until the file/page is refreshed. Either clearloadingon Livewire request completion/failure or dispatch the completion event on the early-return paths.Useful? React with 👍 / 👎.