Skip to content

Remove hardcoded styling and unused colors from DiffRenderer#8

Open
paololeonardi wants to merge 1 commit intotornikegomareli:mainfrom
paololeonardi:remove-hardcoded-styling
Open

Remove hardcoded styling and unused colors from DiffRenderer#8
paololeonardi wants to merge 1 commit intotornikegomareli:mainfrom
paololeonardi:remove-hardcoded-styling

Conversation

@paololeonardi
Copy link
Copy Markdown

@paololeonardi paololeonardi commented Apr 25, 2026

Summary

Removes opinionated layout defaults baked into DiffRenderer so that consumers have full control over spacing and background appearance.
Fixes dark-corner artifacts that appeared when the view was embedded in a grouped List in dark mode.

Changes

  • Remove hardcoded .padding() from DiffRenderer — callers apply it externally
  • Remove fixed Color.appBackground from the ScrollView background, letting the view inherit its container's background
  • Delete PlatformColors.swift and the now-unused Color.appBackground extension, along with the import references in DiffFileView and DiffLineView

Breaking Changes

Consumers that relied on DiffRenderer's built-in padding will need to add .padding() at the call site.

DiffRenderer(diffText: diffText)
   .padding()
DiffRenderer(diffText: diffText)
   .padding(EdgeInsets(top: 8, leading: 16, bottom: 8, trailing: 16))

Testing

Verified manually by embedding DiffRenderer in a grouped List in both light and dark mode — no background artifacts observed.

Before After
Simulator Screenshot - iPhone 17 Pro - 2026-04-25 at 00 54 21 Simulator Screenshot - iPhone 17 Pro - 2026-04-25 at 00 55 01

Summary by CodeRabbit

  • Refactor
    • Removed cross-platform background color abstraction
    • Updated preview styling for diff views
    • Simplified layout by removing padding and background styling from the diff renderer

Drops fixed padding, the hardcoded ScrollView background, and the now-unused
Color.appBackground / PlatformColors, letting consumers control spacing and
background inheritance themselves.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 870c2d25-cc18-4731-b7f3-fa534fa2bd1b

📥 Commits

Reviewing files that changed from the base of the PR and between d773f17 and 8d17ce0.

📒 Files selected for processing (4)
  • Sources/gitdiff/Core/PlatformColors.swift
  • Sources/gitdiff/Views/DiffFileView.swift
  • Sources/gitdiff/Views/DiffLineView.swift
  • Sources/gitdiff/Views/DiffRenderer.swift
💤 Files with no reviewable changes (4)
  • Sources/gitdiff/Views/DiffFileView.swift
  • Sources/gitdiff/Views/DiffLineView.swift
  • Sources/gitdiff/Views/DiffRenderer.swift
  • Sources/gitdiff/Core/PlatformColors.swift

📝 Walkthrough

Walkthrough

The pull request removes the PlatformColors.swift file that abstracted a cross-platform system background color and deletes all references to its Color.appBackground property from preview and view definitions, simplifying background styling across the codebase.

Changes

Cohort / File(s) Summary
Platform Color Abstraction Removal
Sources/gitdiff/Core/PlatformColors.swift
Deleted the platform color abstraction file containing conditional UIKit/AppKit imports and the Color.appBackground computed property.
Preview Background Modifier Cleanup
Sources/gitdiff/Views/DiffFileView.swift, Sources/gitdiff/Views/DiffLineView.swift
Removed .background(Color.appBackground) modifier from preview definitions, allowing default system backgrounds to apply instead.
View Layout Adjustments
Sources/gitdiff/Views/DiffRenderer.swift
Removed padding and background color modifiers from loading, empty-state, and populated content containers in the scrollable view.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 Colors simplified, abstractions fade away,
Platform secrets no longer on display,
Views stand lighter, backgrounds now so plain,
Cleaner code, with less to maintain! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: removing hardcoded styling and unused colors from DiffRenderer, which is the primary focus across deleted files and modified view files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

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