Menu

#85 feat(branches,ui,help): clarify branch sync and divergence indicators

closed
nobody
2026-07-19
2026-07-19
Anonymous
No

Originally created by: theBGuy
Originally owned by: theBGuy

Separate each branch’s upstream push/pull state from its divergence against the default branch so the branch switcher no longer makes work that is merely ahead of the default appear unpushed. The updated indicators and documentation make branch synchronization and comparison status easier to interpret at a glance.

Branch switcher

  • Updates src/features/repository/BranchSwitcher.tsx to display each branch’s own upstream sync state, including commits to push, commits to pull, never-published branches, and branches whose upstream was deleted.
  • Replaces ambiguous default-branch arrows in src/features/repository/BranchSwitcher.tsx with explicit +N/−M divergence counts labeled with the default branch name.
  • Restructures branch rows in src/features/repository/BranchSwitcher.tsx into separate name and detail lines, giving long branch names more room while keeping current-branch checks and relative commit times visible.
  • Preserves pull request and worktree indicators in the detail line of src/features/repository/BranchSwitcher.tsx.

Documentation

  • Clarifies the branch sync and default-branch divergence indicators in README.md.
  • Expands the branch switcher guidance in src/features/help/content.ts to explain upstream push/pull state, remote-deleted branches, and +/− divergence counts.
  • Updates the fetch, update, and push workflow documentation in src/features/help/content.ts to describe sync indicators for every branch.

Changelog

  • Records the changed branch indicators and two-line branch row layout in changelog.d/changed-branch-sync-indicators.md.

Related

Tickets: #86

Discussion

  • Anonymous

    Anonymous - 2026-07-19
     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI security audit · opus · automated


    No security issues found in these changes — the diff is a presentational refactor of branch sync/divergence indicators; all branch/upstream names reach the DOM as React-escaped text content or as title string properties (no dangerouslySetInnerHTML, no dynamic sinks), and the other files are docs/help/changelog copy.


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    Context for reviewers — deliberate choices in this change, with the evidence behind them:

    The problem being fixed. Branch-menu rows previously rendered divergence-vs-default with the same bare ↑/↓ iconography the header's Push/Pull badges use for upstream sync — so "12 ahead of master" read as "12 unpushed commits" (live user report: repeatedly trying to push an already-synced branch). The fix reserves the arrow vocabulary exclusively for a branch's own upstream sync state and re-forms the vs-default divergence as labeled text (+N −M master, U+2212 minus), on a two-line row layout (name on line 1, details on line 2 — also a user request).

    Deliberate calls that might otherwise look like findings:

    1. In-sync branches render NO sync indicator — silence means synced. Never-published and upstream-deleted branches get explicit markers (CloudSlash / CloudX) instead, so the "does this exist on the remote?" question always has an answer.
    2. The sync indicator is skipped entirely when the repo has zero remotes (remoteNames.length === 0) — no push story to report.
    3. The old default-row-only upstreamBehind special case is removed, not lost — the per-row sync indicator generalizes it (and strictly improves it: unpushed commits sitting on the default branch are now visible, which they never were).
    4. Sync arrows carry no explicit text color (inherit row foreground) while divergence stays text-muted-foreground — intentional hierarchy: the actionable state reads one step stronger. Meaning is never color-alone: icon shape + tooltips + the divergence's inline base-name label carry it (WCAG).
    5. Phosphor icons reject a title prop (IconProps has no title; TS2322 confirmed) — tooltips ride wrapper <span title>s, the file's established chip pattern.
    6. CompareBranchCombobox is deliberately unchanged — its ↑/↓ badges are a documented relational frame (its doc comment names the direction, and no upstream-sync indicator exists on that surface to be confused with). The switcher was the only surface mixing the two meanings.
    7. A branch tracking a since-removed remote still shows its raw tracking counts — matches git status; the context menu already hides Push for that case (pre-existing gating, commented at pushable).

    Verification. pnpm build green (tsc -b + bundle), scoped biome check clean, pnpm changelog:preview renders. Live-verified across two repos covering the full state matrix: ↑N to push, ↓N to pull (on the default row), diverged ↑N ↓M beside +N −M main, synced-silent, never-published, upstream-deleted (tooltip verified verbatim), gd/session/* filtered from the menu (hard invariant intact), two-line layout in normal/archived/filtered views, remote-only rows untouched.

    Docs-sync. README + in-app help updated to describe both indicators (the old "ahead/behind vs. the default" phrasing swept from both); marketing site deliberately unchanged — its only ahead/behind claim is the Compare tab's, which is unaffected; changelog fragment included.


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    Sound change: it cleanly separates a branch's own upstream sync state from its divergence-vs-default, which fixes the real "ahead-of-default looks unpushed" confusion, and the docs/changelog are updated to match. Not blocking. I found one behavior-consistency gap and one accessibility gap worth addressing.

    Correctness / consistency

    • should-fixBranchSwitcher.tsx, the new sync indicator (lines ~971–1008) vs. the context-menu upstreamRemote/pushable logic (lines ~861–868). The context menu deliberately treats "tracked on a remote that no longer exists" as non-actionable — upstreamRemote requires remoteNames.includes(branch.upstreamRemote), and its own comment calls out this state ("Hidden … when … tracked on a remote that no longer exists"). The new indicator only gates on remoteNames.length > 0, so for a branch where branch.upstream && !branch.upstreamGone && upstreamAhead > 0 but branch.upstreamRemote is not in remoteNames (upstream tracks a remote that was removed while another remote still exists), the row renders ↑/↓ push/pull arrows implying you can push — yet right-clicking offers no "Push to …". The two surfaces disagree about the same branch. Reuse the menu's guard in the indicator: compute the same remoteNames.includes(branch.upstreamRemote) check and fall through to the CloudSlashIcon/CloudXIcon (or hide) when the tracked remote is missing, so the glyph matches the available action.

    Accessibility

    • should-fixBranchSwitcher.tsx, the never-published (CloudSlashIcon, ~line 1025) and upstream-deleted (CloudXIcon, ~line 1015) states. These two new states convey their meaning through a bare icon plus a title tooltip only — no visible text and no aria-label. Unlike the sibling indicators on the same line (the worktree marker carries the word "worktree", the sync/divergence markers carry numbers), a screen-reader user gets nothing, and title only surfaces on mouse hover, so keyboard users don't see it either. Given the project's keyboard-first / WCAG AA stance, add an aria-label to each span (e.g. aria-label="Local only — never published" / aria-label="Upstream deleted on the remote") so the state has an accessible name independent of hover.

    Nits

    • nitBranchSwitcher.tsx, divergence span (~line 1032): the title now reads ${div.ahead} commits ahead of ${defaultName}, ${div.behind} behind, which produces "0 behind" when only ahead (and "0 commits ahead of main" when only behind). Minor, and it mirrors the pre-existing phrasing, but you could drop the zero side to match the compact +N −M body.

    No test finding — the repo has no frontend test runner, so this UI-only change has no harness to extend. The decision to skip the marketing site is defensible: site/src/pages/index.astro only mentions ahead/behind for the Compare feature, not the switcher indicators, so nothing there is now stale.


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    Round-1 dispositions — all four findings verified against the code; all accepted. Fixes land in the next push.

    1. Removed-remote indicator/menu disagreement (should-fix) — accepted, and it fairly overturns context-comment call [#7]. The disclosed call ("matches git status") didn't account for the sharper point: the row's arrows imply a push the context menu deliberately doesn't offer. Taking the suggested fall-through mechanism with one refinement the guard needs: it diverts only when branch.upstreamRemote is non-null and missing from remoteNames. A branch tracking a local upstream (git branch --track a main) has upstreamRemote = null%(upstream:remotename) is empty and the Rust side filters empty to None (branches.rs) — and its arrows are truthful (sync vs its local base), so it keeps them; the naive includes() check would mislabel it "remote removed". The fall-through marker's tooltip states the fact without claiming an action ("Tracks origin/x, but that remote is no longer configured") because the menu offers neither Push nor Publish in that state (publishable requires !upstream || upstreamGone).

    2. Icon-only states lack accessible names (should-fix) — accepted with an upgrade. The same gap class extends to the sibling the review didn't flag: the sync-arrows span reads as bare numbers ("3 1") to a screen reader — the arrow SVGs are aria-hidden. Fix applies aria-labels mirroring each tooltip to all three indicator spans (sync, cloud markers with role="img", divergence), so every state has an accessible name independent of hover and participates in the row button's accessible name.

    3. Divergence tooltip "0 behind" (nit) + Copilot's missing "commits" — merged into one fix: the title is rebuilt from nonzero parts, each carrying "commits" (13 commits ahead of master, 2 commits behind), matching the body's drop-the-zero-side behavior. Disposition also in the thread reply.

    The in-app help's marker clause gets a matching extension for the new third state (tracked remote no longer configured). CI is green (build 1m14s, fragment, Pages); no Rust changes.


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     

    Related

    Tickets: #7

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    This is a solid, well-reasoned change, and the earlier round's findings are all addressed in the current diff. I verified the types (upstreamRemote, upstreamAhead/Behind, upstreamGone are all non-optional per src/lib/git/types.ts) and the context-menu guard (BranchSwitcher.tsx:861-870) — the new indicator now shares the menu's remoteNames.includes(branch.upstreamRemote) guard exactly, so the two surfaces agree. Not blocking.

    Resolved since last review

    • Indicator/menu disagreement for a removed tracked remote — the sync block now diverts to the muted CloudSlashIcon when branch.upstream && !branch.upstreamGone && branch.upstreamRemote && !remoteNames.includes(branch.upstreamRemote) (lines ~983–998), matching the menu's upstreamRemote guard at 861–867, and correctly keeps a local-tracking branch (upstreamRemote === null) out of that case so its arrows still show. Fixed.
    • Icon-only states lacked an accessible namerole="img" + aria-label added to the never-published and upstream-deleted spans. Fixed.
    • Divergence title read "0 behind" when only ahead — the label is now rebuilt from nonzero parts with singular/plural "commit(s)" (lines ~1061–1085). Fixes both the prior nit and Copilot's "missing the word commits" note.

    Accessibility

    • nitBranchSwitcher.tsx, the sync-arrows span (~line 1017) and, to a lesser degree, the divergence span (~line 1073). These carry aria-label but no role, so they stay role="generic", for which aria-label is spec-prohibited and not reliably exposed by assistive tech — unlike the sibling icon-only spans that correctly use role="img". The sync span is the one that actually degrades: its arrow SVGs are aria-hidden, so if the label is dropped a screen reader reads just "3 1" with no push/pull context. The divergence span degrades better since "main" is visible text. Add role="img" to the sync-arrows span (and, for consistency, the divergence span) so the label is announced reliably.

    No other issues. The removed default-branch upstreamBehind special case is genuinely subsumed by the per-row sync indicator (and improved — it now surfaces ahead as well), and div is guarded by (div.ahead > 0 || div.behind > 0) so the default row never shows a vacuous self-divergence. README, help content, and the changelog fragment all match the new behavior.


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    Round-2 disposition — the one nit is accepted and applied; in the next push.

    The finding is correct per the ARIA spec: aria-label on a role="generic" element is prohibited naming and not reliably exposed, and the sync-arrows span is the case that genuinely degrades (its arrow SVGs are aria-hidden, leaving bare "3 1"). Both the sync-arrows span and the divergence span now carry role="img" alongside their aria-labels, matching the icon-only siblings — every indicator on the row uses the same pattern. A short comment at the sync span records why the role is load-bearing. Verified: pnpm build green, scoped biome clean.

    (Also disclosed in the round-1 push, for completeness: the rebuilt divergence tooltip gained singular/plural handling — 1 commit ahead, not 1 commits — caught in orchestrator diff review.)


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    Verified the current diff against the source: Branch.upstreamAhead/upstreamBehind (number), upstreamGone (boolean), upstreamRemote (string | null), and lastCommitDate (string) are all non-optional in src/lib/git/types.ts, and the sync indicator's removed-remote guard (branch.upstream && !branch.upstreamGone && branch.upstreamRemote && !remoteNames.includes(...)) is the exact complement of the context menu's upstreamRemote computation at lines 861–867, so the row and menu still agree.

    This is a sound, well-scoped change that cleanly separates upstream sync state from vs-default divergence, and the docs (README, help, changelog) match the new behavior. Not blocking.

    Resolved since last review

    • Sync-arrows / divergence spans lacked role — both spans now carry role="img" alongside aria-label (BranchSwitcher.tsx ~1017 and ~1082), so the labels are reliably announced instead of sitting on a role="generic" element where aria-label is spec-prohibited. This was the only open finding; fixed.

    Accessibility

    • nitBranchSwitcher.tsx, the divergence label (~line 1069). When a branch is only behind the default (div.ahead === 0, div.behind > 0), the label is built from just ${div.behind} commit(s) behind with no reference branch, and since the span is now role="img" its visible {defaultName} child is suppressed for AT — a screen-reader user hears "2 commits behind" with nothing to anchor it to. (The both-sides and ahead-only cases name the default fine.) Concrete fix: include the name in the behind-only case, e.g. push `${div.behind} commit${div.behind === 1 ? "" : "s"} behind${div.ahead > 0 ? "" : ` ${defaultName}`}` so it reads "2 commits behind main" when standalone but stays unrepeated when paired with the ahead part.

    No other issues. Copilot's "missing the word commits" note on the tooltip is already addressed — the label is rebuilt from nonzero parts with singular/plural commit(s). The removed default-branch upstreamBehind special case is genuinely subsumed by the general per-row sync indicator, and the remoteless-repo skip plus the in-sync silence are consistent with the context-menu gating.


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    Round-3 disposition — accepted and applied exactly as suggested; in the next push.

    The finding is the composition of two earlier correct fixes (the nonzero-parts label rebuild × role="img" suppressing the visible {defaultName} child for AT), which left the behind-only case unanchored. The behind part now appends the default branch's name when — and only when — the ahead part isn't already naming it: "2 commits behind main" standalone, "13 commits ahead of main, 2 commits behind" when paired. A short comment at the site records why the conditional anchor is load-bearing. Verified: pnpm build green, scoped biome clean.


    Posted by GitDesktop — automated agent comment, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated


    This change cleanly separates a branch's own upstream push/pull state from its divergence-vs-default in the switcher rows, reworks the row into a two-line layout, and updates README/help/changelog to match. I verified the types (upstreamAhead/upstreamBehind/upstreamGone/upstreamRemote/lastCommitDate are all non-optional or nullable exactly as the code assumes), the div source (divByNameBranchDivergence, vs-default), and that the removed-remote guard (branch.upstream && !branch.upstreamGone && branch.upstreamRemote && !remoteNames.includes(...)) is the precise complement of the context-menu upstreamRemote computation at lines 861–867 — row and menu still agree. The change is sound and nothing is blocking.

    Resolved since last review

    • Behind-only divergence label lacked a reference branch (BranchSwitcher.tsx ~line 1077) — the current diff appends the default's name in the behind-only case (behind${div.ahead > 0 ? "" : \ ${defaultName}`}`), so a screen reader now hears "2 commits behind main" standalone while the paired case stays "…ahead of main, 2 commits behind" (unrepeated). Confirmed fixed in the shown code.

    Notes

    • Copilot's :1033 tooltip finding ("missing the word 'commits' before 'behind'") is stale — the divergence label is now rebuilt from nonzero parts each carrying commit${…=== 1 ? "" : "s"}, so it reads "2 commits behind". Not an issue.
    • Docs-sync: README and in-app help were updated; the marketing site has no matching switcher-indicator claim to refresh (its ahead/behind copy at site/.../index.astro describes the separate branch-comparison surface), so nothing stale there.

    No further issues — the sync/divergence split, the remoteless-repo skip, the in-sync silence, and the role="img" + aria-label pattern across all indicator spans are all consistent.


    Posted by GitDesktop — AI output, verify before acting on it.

     
  • Anonymous

    Anonymous - 2026-07-19

    Ticket changed by: theBGuy

    • status: open --> closed
     

Log in to post a comment.