Skip to content

Inset Settings sidebar top and widen the column#373

Merged
FuJacob merged 1 commit into
mainfrom
fix/settings-sidebar-width
May 28, 2026
Merged

Inset Settings sidebar top and widen the column#373
FuJacob merged 1 commit into
mainfrom
fix/settings-sidebar-width

Conversation

@FuJacob
Copy link
Copy Markdown
Owner

@FuJacob FuJacob commented May 28, 2026

First sidebar row was flush against the title bar; the column was also too narrow so labels like 'Apple Intelligence' truncated to 'Apple I...'. Inserts a clear-color spacer at the top of the List and bumps the column width to min 240 / ideal 260 / max 320.

Greptile Summary

This PR fixes two visual issues in the Settings sidebar: the first row was flush against the window title bar, and the column was too narrow for longer labels like "Apple Intelligence".

  • A Color.clear spacer row (8 pt) is inserted at the top of the List with .selectionDisabled() to add top breathing room without conflicting with .listStyle(.sidebar).
  • The navigationSplitViewColumnWidth is bumped from min: 200 / ideal: 220 / max: 260 to min: 240 / ideal: 260 / max: 320 to prevent label truncation.

Confidence Score: 4/5

Safe to merge; changes are cosmetic layout tweaks with a single minor accessibility gap in the spacer row.

Both changes are narrow and purposeful. The wider column width is straightforward. The Color.clear spacer row works correctly for sighted users, but it is not marked .accessibilityHidden(true), so VoiceOver will encounter an unnamed, empty list element when navigating the sidebar.

Cotabby/UI/Settings/SettingsSidebarView.swift — the spacer row needs .accessibilityHidden(true) before the accessibility story is complete.

Important Files Changed

Filename Overview
Cotabby/UI/Settings/SettingsSidebarView.swift Adds an 8 pt clear spacer row at the top of the sidebar List to prevent the first row sitting flush against the title bar, and widens the column width (min 240 / ideal 260 / max 320). The spacer row is selection-disabled but not accessibility-hidden, leaving it reachable by VoiceOver.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[SettingsSidebarView body] --> B[List with selection binding]
    B --> C[Color.clear spacer row]
    B --> D[ForEach SettingsSidebarSection.allCases]
    D --> E{section has rows?}
    E -- yes --> F{section has title?}
    F -- yes --> G[Section with header]
    F -- no --> H[Section no header]
    G --> I[ForEach rows]
    H --> I
    B --> J[.listStyle .sidebar]
    J --> K[navigationSplitViewColumnWidth min 240 ideal 260 max 320]
Loading

Fix All in Codex Fix All in Claude Code

Reviews (1): Last reviewed commit: "Inset sidebar top + widen so labels stop..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

@FuJacob FuJacob merged commit e004831 into main May 28, 2026
4 checks passed
@FuJacob FuJacob deleted the fix/settings-sidebar-width branch May 28, 2026 10:55
Comment on lines +22 to +25
Color.clear
.frame(height: 8)
.listRowBackground(Color.clear)
.selectionDisabled()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Adding .accessibilityHidden(true) prevents VoiceOver from landing on this invisible placeholder row when the user navigates the sidebar.

Suggested change
Color.clear
.frame(height: 8)
.listRowBackground(Color.clear)
.selectionDisabled()
Color.clear
.frame(height: 8)
.listRowBackground(Color.clear)
.selectionDisabled()
.accessibilityHidden(true)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Codex Fix in Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant