Menu ▾ ▴

#86 feat(sync,ui): show ahead and behind counts on sync buttons

closed
nobody
enhancement (85)
2026-07-19
2026-07-19
Anonymous
No

Originally created by: theBGuy
Originally owned by: theBGuy

Surface branch synchronization counts directly on the actions that use them, so users can immediately see what will be pushed or pulled without interpreting separate badges. Reorders the controls to prioritize Push and Pull while preserving clear disabled-state guidance and accessible labels.

Sync controls

  • Updates src/features/repository/SyncControls.tsx to display ahead counts on the Push or Publish branch button and behind counts on the Pull button.
  • Reorders the header controls in src/features/repository/SyncControls.tsx to Push, Pull, Pull options, and Fetch.
  • Adds matching tooltips and accessible labels that describe commit counts, detached HEAD state, missing upstreams, and diverged branches.
  • Removes the standalone animated ahead/behind badges and explicitly manages button-group borders and focus styling across the wrapped controls.

Documentation and changelog

  • Updates the sync feature help text in src/features/help/content.ts to describe the reordered controls and inline counts.
  • Updates the syncing overview in README.md to document counts on the Push and Pull buttons.
  • Adds changelog.d/changed-sync-header-buttons.md describing the control reorder and count placement.
  • Updates the sync indicator reference in src/features/repository/BranchSwitcher.tsx to match the new header button terminology.

Related

Tickets: #89

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 UI-only refactor of the sync buttons (ahead/behind counts moved onto the Push/Pull buttons); all interpolated values (aheadCount, behindCount, head?.upstream) are rendered as React text/attribute content, which is auto-escaped — no dangerouslySetInnerHTML, no injection sink, no auth/secret/data-flow surface touched.

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

     

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    Context for reviewers — deliberate calls in this change, with the reasoning on record:

    1. Seam ownership moved to the call site. All four segments (Push, Pull, Pull-options caret, Fetch) are wrapped in <span title> wrappers because tooltip text must survive natively-disabled buttons (which swallow their own title). Those spans have no data-slot, so they opt out of the vendored ButtonGroup's border-collapse/rounding child selectors (*:data-slot:* / [&>[data-slot]~[data-slot]]:*) — relying on the primitive is what made the caret visually fuse to Fetch in an intermediate revision. The explicit rounded-*-none border-l-0 classes on each Button plus focus-visible:relative focus-visible:z-10 (replacing the group's unreachable *:focus-visible:z-10) are therefore load-bearing, documented at the group. Suggestions to "let ButtonGroup handle this" would reintroduce the fused-to-Fetch bug.
    2. One description string per button feeds BOTH the wrapper title and the Button aria-label (diverged → reason + behind count + remedy; detached; no-upstream; plain count; else undefined = no tooltip, no aria-label — the visible label suffices). The visible count spans are aria-hidden so the accessible name stays the composed sentence, and every string starts with the visible label (WCAG 2.5.3 label-in-name).
    3. The behind count dims with the disabled Pull button when diverged (~2.7:1). This is the platform disabled convention and WCAG-exempt (inactive component); the diverged tooltip/aria string carries the same number at full contrast as the mitigation. Please don't flag the dimmed count as a contrast failure.
    4. Counts appear without a transition — deliberate; matches category peers (GitHub Desktop), and the app's motion register is state-not-decoration. The removed standalone badges' scale-in went with them.
    5. Vocabulary dual-shape is deliberate: branch-menu rows (PR [#85]) render fused ↑8 icon+number pairs (compact row indicators); the header renders ↑ Push … 8 (labeled action button with a trailing muted count). Different component classes, same data and tokens — reviewed by a three-lens design panel and accepted.
    6. The counts are the branch's own upstream sync numbers (head.ahead/behind), exactly what the old badges showed — no semantic change, presentation only. [#85]'s sync-vs-divergence split is untouched.
    7. Marketing site untouched — it makes no button-order/badge claims (grepped). README + in-app guide updated in this diff. One follow-up already staged locally for the next push: the guide section heading # Fetch, pull, push & conflicts → # Push, pull, fetch & conflicts (display-only; the section id/label are separate).

    Verification: pnpm build green pre- and post-revision; tsc -b + scoped biome clean; live-verified in the dev app — ahead-8 state rendered ↑ Push 8, post-revision cluster shows uniform 1px seams with rounded outer caps only, and the wrapper-span tooltip mechanism confirmed rendering on hover.

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

     

    Related

    Tickets: #85

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    🤖 GitDesktop AI review · opus · automated

    Summary

    This reworks SyncControls to move the ahead/behind counts onto the Push and Pull buttons (replacing the standalone animated badges), reorders the header to Push / Pull / Fetch, and manages the ButtonGroup seams manually because each button is now wrapped in a title span. The change is sound and non-blocking — the logic is careful and the accessibility handling (shared title/aria-label, aria-hidden count digits, label-in-name ordering) is well thought through. Only minor nits below.

    Correctness (verified clean — no action needed)

    The label strings interpolate ${head?.upstream}, and upstream is typed string | null — but every interpolation is gated behind a nonzero count or diverged, and git only reports ahead/behind > 0 (and hence diverged) when a live upstream exists, so a literal "null" can't surface here. The manual border-l-0 seam classes correctly replace ButtonGroup's [&>[data-slot]~[data-slot]]:border-l-0, which no longer matches now that the direct children are data-slot-less spans; borders collapse and there's no gap since the group adds none. Unused Badge/motion imports were removed cleanly. Frontend has no test runner per project posture, so no missing-test finding.

    Readability

    nit — SyncControls.tsx, the ButtonGroup comment (~lines 256–257): it says the call site owns "rounded outer caps," but the base Button is square (rounded-none at the cva root and in the sm size variant), so the rounded-r-none / rounded-l-none / rounded-none classes are inert no-ops — only the border-l-0 joins are actually load-bearing. A future maintainer reading this could believe corner-rounding is being managed when it isn't. Suggest trimming the "rounded outer caps" phrasing to reflect that the group is square and only the left-border collapse matters.

    nit — src/features/help/content.ts, syncing section (lines 491–497): the intro now reads "Push / Pull / Fetch," but the bullet list immediately below still explains them in the old Fetch → Pull → Push order. Given the reorder's stated rationale ("most-used action first"), reorder the bullets to Push / Pull / Fetch so the guide is internally consistent.

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

     

  • Anonymous

    Anonymous - 2026-07-19

    Originally posted by: theBGuy

    Round-1 dispositions (fixes land in the next push, alongside the guide-heading reorder already announced above):

    • AI review nit 1 (inert rounded-* classes / "rounded outer caps" comment) — accepted, with an upgrade. Verified against the vendored cva: rounded-none at the Button root and in the sm variant, so the corner classes were indeed no-ops (and the context comment's "rounded outer caps" phrasing above was wrong — the caps are square, and were before this PR too). Rather than only rewording the comment, the inert rounded-r-none / rounded-none / rounded-l-none utilities are removed from all four buttons — dead classes claiming un-honored intent are exactly the trap the comment warns about. The comment now states the Button is square and only the border-l-0 joins are load-bearing. No visual change (verified no-op removal; tsc -b + scoped biome green).
    • AI review nit 2 (guide bullets in old Fetch→Pull→Push order) — accepted. Bullets reordered to Push / Pull / Fetch, matching the intro sentence and the header's left-to-right order.
    • Copilot's upstreamGone finding — accepted and fixed (see the resolved thread): dedicated gone-upstream arm in pullDescription, distinguishing configured-but-dead from never-published.

    The review's correctness section (no "null" interpolation reachable; seam mechanics; import cleanup) matches my own trace — nothing further to add there.

    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 is a clean, well-reasoned refactor: it moves the ahead/behind counts onto the Push/Pull buttons, reorders the group to Push / Pull / Fetch, and manages the ButtonGroup seams manually because each segment is now wrapped in a title span. I traced the label logic, the seam classes, the accessibility handling, and the import cleanup against the actual code — it's sound and nothing is blocking.

    Verification notes (no action needed)

    • ${head?.upstream} interpolation is safe. Every string that interpolates the upstream ref is gated behind a state where the backend guarantees a non-null upstream: aheadLabel/behindLabel require ahead>0/behind>0 (which git only reports against a live tracking ref), diverged requires both counts >0, and the new upstreamGone arm fires exactly when the upstream is configured-but-dead (upstream non-null by contract, per BranchHead docs). No "null" can surface.
    • Removing rounded-r-none/rounded-none/rounded-l-none is a genuine no-op. Confirmed in src/components/ui/button.tsx: rounded-none is in the cva root and in the sm size variant, so those utilities were inert. border-l-0 on the three inner joins (Pull, Pull-options caret, Fetch) is the only load-bearing seam class, and Push correctly omits it as the first segment — the comment now matches reality.
    • upstreamGone Pull arm is reachable and correct. hasUpstream = Boolean(head?.upstream) && !head?.upstreamGone is false when gone, so the Pull button is disabled, and the dedicated arm (ahead of !hasUpstream) explains why rather than falsely claiming "never published."
    • Import cleanup is clean. Grepped the file: no remaining references to the removed m/AnimatePresence/Badge/quickTransition, and ArrowUpIcon/ArrowDownIcon are still both imported and used.
    • Docs stay in sync. README, the help syncing section (heading, intro, and bullets all now in Push/Pull/Fetch order), and the changelog fragment are consistent with the new UI.

    Resolved since last review

    • Inert rounded-* classes / "rounded outer caps" comment (previous nit 1) — the current diff removes those utilities from all four buttons and rewrites the comment to state the Button is square and only border-l-0 is load-bearing.
    • Help bullets in old Fetch→Pull→Push order (previous nit 2) — bullets are now Push / Pull / Fetch, matching the intro and header order.
    • upstreamGone treated like an untracked branch (Copilot, SyncControls.tsx:165) — now has its own arm producing "upstream <ref> was deleted on the remote (likely merged); use Publish branch to recreate it."

    No new findings.

    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.

MongoDB Logo MongoDB