Repository navigation
fix(ui,checks): clip Select picker rows through a shared helper - #259
Conversation
Long branch, workflow, and environment names ran past the Select popup edge
instead of ellipsizing: a bare `truncate` child never engages because
ItemText's `min-width: auto` floor grows with its nowrap content, and the
item-level `clipTitle` workaround measured an element that never overflows.
- `src/components/select-clip-text.tsx`: new `SelectClipText` wraps popup-row
text in a `w-0 min-w-full truncate` span and titles it on hover only when
the text is actually clipped.
- Call sites converted: the merge, rebase-onto, cherry-pick-onto and worktree
branch pickers; repo settings (general, GitLab, Bitbucket, Pages, secrets
scope); the run workflow/pipeline dialog; and the line-endings select. Each
closed field now carries `onMouseEnter={clipTitleFromText}` on its
`SelectValue`.
- `scripts/check-banned-patterns.mjs`: new `select-item-clip-title` guard
fails on both superseded shapes (a `clipTitle` handler under a `SelectItem`,
a bare `block truncate` child), covered by `scripts/checks.test.mjs`.
- `.claude/skills/gd-conventions/SKILL.md` records the idiom, plus a changelog
fragment for the picker behavior change.
Deploying gitdesktop with
|
| Latest commit: |
c6cb7ca
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4a5e34bd.gitdesktop.pages.dev |
| Branch Preview URL: | https://fix-select-picker-clip-toolt.gitdesktop.pages.dev |
|
Context for reviewers — author notes, on the record before review fires:
Verification: Posted by GitDesktop — automated agent comment, verify before acting on it. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes consistently apply a single verified clipping strategy across call sites and add a guard + tests to prevent regressions.
Pull request overview
This pull request fixes long labels overflowing (instead of truncating) in Base UI Select popups and triggers by introducing a single, reliable clipping + hover-title helper and routing all affected picker call sites through it. It also adds a guard + tests to prevent the two previously “dead” truncation/tooltip patterns from being reintroduced.
Changes:
- Added
SelectClipTextto make Select popup row text actually ellipsize and only set atitlewhen clipped. - Updated Select picker call sites to use
SelectClipTextfor rows andonMouseEnter={clipTitleFromText}onSelectValuefor the closed field. - Added a banned-patterns guard (
select-item-clip-title) with unit tests, plus updated conventions docs and a changelog fragment.
File summaries
| File | Description |
|---|---|
| src/components/select-clip-text.tsx | Introduces the shared SelectClipText helper that self-bounds row text and sets hover title only when clipped. |
| src/features/settings/GitSection.tsx | Converts line endings picker row text + closed-field hover title to the shared helper/handler. |
| src/features/repository/WorktreesDialog.tsx | Replaces item-level tooltip logic with SelectClipText and adds closed-field hover title handling. |
| src/features/repository/RebaseOntoDialog.tsx | Routes both base pickers’ rows through SelectClipText and adds SelectValue hover title handling. |
| src/features/repository/BranchMergePickerDialog.tsx | Converts merge branch picker rows to SelectClipText and adds SelectValue hover title handling. |
| src/features/repo-settings/SecretsSection.tsx | Switches environment scope picker rows to SelectClipText and adds closed-field hover title handling. |
| src/features/repo-settings/PagesSection.tsx | Converts both Pages branch pickers’ rows to SelectClipText and adds closed-field hover title handling. |
| src/features/repo-settings/GitLabGeneralSection.tsx | Converts default-branch picker rows to SelectClipText and adds closed-field hover title handling. |
| src/features/repo-settings/GeneralSettingsSection.tsx | Converts commit template/default branch picker rows to SelectClipText and adds closed-field hover title handling. |
| src/features/repo-settings/BitbucketSchedulesSection.tsx | Converts schedule branch picker rows to SelectClipText and adds closed-field hover title handling. |
| src/features/repo-settings/BitbucketGeneralSection.tsx | Converts main branch picker rows to SelectClipText and adds closed-field hover title handling. |
| src/features/history/HistoryDialogs.tsx | Converts cherry-pick destination rows to SelectClipText and adds closed-field hover title handling. |
| src/features/actions/RunWorkflowDialog.tsx | Converts workflow/pipeline picker rows to SelectClipText and adds closed-field hover title handling. |
| scripts/check-banned-patterns.mjs | Adds the select-item-clip-title check to prevent reintroducing the two superseded Select truncation/tooltip shapes. |
| scripts/checks.test.mjs | Adds tests covering both matching arms and the intended non-matches/exemptions for the new guard. |
| .claude/skills/gd-conventions/SKILL.md | Updates the repo conventions doc to point contributors at SelectClipText + the SelectValue hover handler. |
| changelog.d/changed-select-picker-clip-affordances.md | Documents the user-visible behavior change (ellipsis + hover-to-see-full-name) across the affected pickers. |
Review details
- Files reviewed: 17/17 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
🤖 GitDesktop AI security audit · No security issues found in these changes. The diff is UI-only: Posted by GitDesktop — AI output, verify before acting on it. |
|
Warning Review limit reachedNext included review available in 26 minutes. View limit detailsLimit details: You’ve used all 2 included reviews currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe change adds ChangesSelect picker clipping
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🔵 Low · up to This PR centralizes Select clipping and hover titles across picker rows and closed fields. A bounded source-check false positive and stale closed-field titles after selection changes still require owner follow-up, so the change is mergeable with explicit awareness. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 15 files. (2 skipped: 2 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8a63fe56-621e-424a-bc81-22d2addda28e
📒 Files selected for processing (17)
.claude/skills/gd-conventions/SKILL.mdchangelog.d/changed-select-picker-clip-affordances.mdscripts/check-banned-patterns.mjsscripts/checks.test.mjssrc/components/select-clip-text.tsxsrc/features/actions/RunWorkflowDialog.tsxsrc/features/history/HistoryDialogs.tsxsrc/features/repo-settings/BitbucketGeneralSection.tsxsrc/features/repo-settings/BitbucketSchedulesSection.tsxsrc/features/repo-settings/GeneralSettingsSection.tsxsrc/features/repo-settings/GitLabGeneralSection.tsxsrc/features/repo-settings/PagesSection.tsxsrc/features/repo-settings/SecretsSection.tsxsrc/features/repository/BranchMergePickerDialog.tsxsrc/features/repository/RebaseOntoDialog.tsxsrc/features/repository/WorktreesDialog.tsxsrc/features/settings/GitSection.tsx
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Cloudflare Pages
⚠️ CI failures not shown inline (2)
GitHub Actions: changelog / 0_fragment.txt: fix(ui,checks): clip Select picker rows through a shared helper
Conclusion: failure
##[group]Run node scripts/changelog-draft.mjs --check
�[36;1mnode scripts/changelog-draft.mjs --check�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
##[error]two or more em-dashes in one bullet reads as a paired "— aside —" interrupter — recast with commas or parentheses; quoted UI copy is exempt inside backticks, **bold**, or "quotes" (see CLAUDE.md "Reader-facing prose outside the blog")
GitHub Actions: changelog / fragment: fix(ui,checks): clip Select picker rows through a shared helper
Conclusion: failure
##[group]Run node scripts/changelog-draft.mjs --check
�[36;1mnode scripts/changelog-draft.mjs --check�[0m
shell: /usr/bin/bash -e {0}
##[endgroup]
##[error]two or more em-dashes in one bullet reads as a paired "— aside —" interrupter — recast with commas or parentheses; quoted UI copy is exempt inside backticks, **bold**, or "quotes" (see CLAUDE.md "Reader-facing prose outside the blog")
🧰 Additional context used
📓 Path-based instructions (2)
- **Keyboard-first, WCAG AA** — wire arrow-key nav for any new selectable list in the
📄 CodeRabbit inference engine (CLAUDE.md)
Files:
src/components/select-clip-text.tsxsrc/features/settings/GitSection.tsxsrc/features/repo-settings/BitbucketSchedulesSection.tsxsrc/features/repository/RebaseOntoDialog.tsxsrc/features/repository/BranchMergePickerDialog.tsxsrc/features/history/HistoryDialogs.tsxsrc/features/actions/RunWorkflowDialog.tsxsrc/features/repo-settings/BitbucketGeneralSection.tsxsrc/features/repo-settings/GitLabGeneralSection.tsxsrc/features/repo-settings/GeneralSettingsSection.tsxsrc/features/repo-settings/SecretsSection.tsxsrc/features/repository/WorktreesDialog.tsxsrc/features/repo-settings/PagesSection.tsx
- **Conventional Commits** with a scope: `feat(github,issues): …`, `fix(diff): …`.
📄 CodeRabbit inference engine (CLAUDE.md)
Files:
changelog.d/changed-select-picker-clip-affordances.mdsrc/components/select-clip-text.tsxsrc/features/settings/GitSection.tsxsrc/features/repo-settings/BitbucketSchedulesSection.tsxsrc/features/repository/RebaseOntoDialog.tsxsrc/features/repository/BranchMergePickerDialog.tsxsrc/features/history/HistoryDialogs.tsxsrc/features/actions/RunWorkflowDialog.tsxsrc/features/repo-settings/BitbucketGeneralSection.tsxsrc/features/repo-settings/GitLabGeneralSection.tsxsrc/features/repo-settings/GeneralSettingsSection.tsxsrc/features/repo-settings/SecretsSection.tsxscripts/checks.test.mjsscripts/check-banned-patterns.mjssrc/features/repository/WorktreesDialog.tsxsrc/features/repo-settings/PagesSection.tsx
🧠 Learnings (3)
📚 Learning: 2026-08-17T06:37:10.176Z
Learnt from: theBGuy
Repo: theBGuy/GitDesktop PR: 214
File: changelog.d/fixed-worktree-removal-stays-visible.md:0-0
Timestamp: 2026-08-17T06:37:10.176Z
Learning: In GitDesktop changelog fragments, describe the current user-visible behavior and any intentional control exceptions. Do not narrate prior defects or internal implementation history, including wording about what a previous UI state “used to” do.
Applied to files:
changelog.d/changed-select-picker-clip-affordances.md
📚 Learning: 2026-08-15T16:52:49.221Z
Learnt from: theBGuy
Repo: theBGuy/GitDesktop PR: 209
File: changelog.d/fixed-stash-mid-op-guard.md:1-5
Timestamp: 2026-08-15T16:52:49.221Z
Learning: GitDesktop changelog entries should describe the user-facing benefit or behavior change concisely, without implementation-level epistemic hedging or internal details such as best-effort detection caveats. Document technical limitations and guarantees in the relevant source-code comments instead.
Applied to files:
changelog.d/changed-select-picker-clip-affordances.md
📚 Learning: 2026-08-16T06:23:57.614Z
Learnt from: theBGuy
Repo: theBGuy/GitDesktop PR: 212
File: changelog.d/fixed-update-branch-async.md:0-0
Timestamp: 2026-08-16T06:23:57.614Z
Learning: In GitDesktop changelog fragments, retain visible UI details—such as control states, hover text, tabs, and header placement—when they clarify intentional user-facing behavior. Omit internal implementation mechanisms, including polling details and query comparisons.
Applied to files:
changelog.d/changed-select-picker-clip-affordances.md
🔇 Additional comments (14)
src/components/select-clip-text.tsx (1)
1-19: LGTM!.claude/skills/gd-conventions/SKILL.md (1)
98-103: LGTM!src/features/actions/RunWorkflowDialog.tsx (1)
4-4: LGTM!Also applies to: 23-23, 201-207, 231-236
src/features/history/HistoryDialogs.tsx (1)
4-4: LGTM!Also applies to: 24-24, 270-275
src/features/repository/BranchMergePickerDialog.tsx (1)
8-8: LGTM!Also applies to: 28-28, 217-222
src/features/repository/RebaseOntoDialog.tsx (1)
3-3: LGTM!Also applies to: 22-22, 175-180, 197-202
src/features/repository/WorktreesDialog.tsx (1)
18-18: LGTM!Also applies to: 45-45, 883-894, 915-923
src/features/settings/GitSection.tsx (1)
3-3: LGTM!Also applies to: 12-12, 237-242
src/features/repo-settings/BitbucketGeneralSection.tsx (1)
4-4: LGTM!Also applies to: 17-17, 274-276
src/features/repo-settings/BitbucketSchedulesSection.tsx (1)
6-6: LGTM!Also applies to: 19-19, 342-344
src/features/repo-settings/GeneralSettingsSection.tsx (1)
5-5: LGTM!Also applies to: 20-20, 183-185, 361-363
src/features/repo-settings/GitLabGeneralSection.tsx (1)
4-4: LGTM!Also applies to: 17-17, 297-299
src/features/repo-settings/PagesSection.tsx (1)
5-5: LGTM!Also applies to: 20-20, 146-148, 299-301
src/features/repo-settings/SecretsSection.tsx (1)
5-5: LGTM!Also applies to: 17-17, 146-148
|
🤖 GitDesktop AI review · SummaryRoutes every repo-content Select popup row through a new shared Guard & tests
Docs & copy
Readability
Recorded decisions I'm not re-flagging: the Posted by GitDesktop — AI output, verify before acting on it. |
|
Dispositions for the AI review (each verified against source; changes land in the next push):
Posted by GitDesktop — automated agent comment, verify before acting on it. |
- `scripts/check-banned-patterns.mjs`: temper both `select-item-clip-title` patterns with `(?!</SelectItem\b)` so each scan stops at the item's closing tag. Previously a stacked picker could pair one Select's last item with the next Select's `SelectValue` trigger handler once the gap tightened; the comment now records the one remaining false positive (a self-bounded `max-w` clip span inside an item) and points at the allowlist as its remedy. - `scripts/checks.test.mjs`: the stacked-picker fixture, which fired before the tempered step, now asserts clean, and the self-bounded span is asserted as the known false positive instead of as legal. - `changelog.d/changed-select-picker-clip-affordances.md`: lead with the behavior, note the closed-field hover, and add the sites the first draft missed (cherry-pick, line-endings, commit-message model pickers). - `gd-conventions` skill: state that `SelectClipText` must be the row's sole child, since a sibling child reintroduces the shrink-refusing layout.
|
🤖 GitDesktop AI review · Fix round verified: the tempered Guard
Resolved since last review
CodeRabbit's stale- Verdict: no blocking issues — remaining items are non-blocking; merge when ready Posted by GitDesktop — AI output, verify before acting on it. |
|
Fix-round disposition: the message-string collateral is accepted — with the tempered step in, "(stacked pickers)" named a false-positive class that can no longer occur while the surviving one went unnamed. The Posted by GitDesktop — automated agent comment, verify before acting on it. |
The banned-pattern message told authors to allowlist "stacked pickers" without saying what makes such a pairing legitimate. Point instead at the real shape — a self-bounded clip span inside a rich row, whose own max-w keeps the item-level handler live — so the allowlist rationale can be judged rather than guessed at.
|
🤖 GitDesktop AI review · The one-line delta since my last review applies the Resolved since last review
Nothing further to raise. CodeRabbit's stale- Verdict: no blocking issues — remaining items are non-blocking; merge when ready Posted by GitDesktop — AI output, verify before acting on it. |
Long branch, workflow, and environment names in Select popups ran past the row instead of ellipsizing, and the per-site attempts to fix that never engaged: the popup hard-clips overflow while
ItemText'smin-width: autofloor grows with its nowrap content, so a baretruncatechild can't shrink and an item-levelclipTitlehandler measures an element that never overflows. This replaces both dead shapes with oneSelectClipTextcomponent, routes every picker through it, and adds achecksguard so the superseded shapes can't come back.Shared helper
SelectClipTextinsrc/components/select-clip-text.tsx— aw-0 min-w-full truncatespan withonMouseEnter={clipTitleFromText}, so the row collapses the min-width floor, re-expands to the row's real width (including under call-site popupmax-woverrides), and titles on hover only when actually clipped. The doc comment records the sole-child constraint:min-w-fullclaimsItemText's whole content box, so a sibling icon or badge would be pushed past the clip edge.Picker call sites
onMouseEnter={clipTitleFromText}to the closed field'sSelectValueacross the branch pickers:BranchMergePickerDialog.tsx,RebaseOntoDialog.tsx(both new-base and old-base),WorktreesDialog.tsx(base and existing-branch), andHistoryDialogs.tsx(cherry-pick destination).GeneralSettingsSection.tsx(commit-message template and default branch),BitbucketGeneralSection.tsx,BitbucketSchedulesSection.tsx,GitLabGeneralSection.tsx,PagesSection.tsx(both the disabled and enabled branch pickers), andSecretsSection.tsx(environment scope) — plusRunWorkflowDialog.tsx(workflow and pipeline) andsettings/GitSection.tsx(line endings).clipTitlehandlers and their explanatory comments inGeneralSettingsSection.tsx,SecretsSection.tsx, andWorktreesDialog.tsx, and the inert<span className="block truncate">children inGeneralSettingsSection.tsx,PagesSection.tsx, andGitSection.tsx.Guard and tests
select-item-clip-titlecheck toscripts/check-banned-patterns.mjswith twoPAIR_GAP-bounded patterns — aclipTitletoken following a<SelectItem, and ablock truncatechild of one — with the message pointing atSelectClipTextand an allowlist escape hatch for stacked pickers. The comment records why the gap is[\s\S](prop expressions carry=>arrows) and that the closest real forward gap today measures 379 normalized chars.scripts/checks.test.mjs: the wrapped-props and inner-span variants flag, while the convertedSelectClipTextidiom,SelectValuetrigger handlers, non-Select rowclipTitle, and a self-boundedmax-w-64 truncatespan stay clean; a third test pins the vendored-uiexemption.Documentation
.claude/skills/gd-conventions/SKILL.mdso it namesSelectClipText, both failure shapes the guard catches, and theSelectValuehandler for the closed field.changelog.d/changed-select-picker-clip-affordances.mddescribing the hover-to-see-full-name behavior across the merge, rebase, worktree, settings, workflow, pipeline, and environment pickers.Summary by CodeRabbit
New Features
Documentation