Remove padding from comment separator views #326
Conversation
- Removed custom Divider with padding from CommentRow - Changed listRowSeparator from .hidden to .visible - Native separator automatically adjusts width based on comment hierarchy
WalkthroughThe Comments component UI receives visual and accessibility refinements. List row separators transition from hidden to visible, layout insets are restructured with level-based leading indentation, an internal Divider is removed, and accessibility hint text is updated. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Areas requiring attention:
Poem
Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Features/Comments/Sources/Comments/CommentsComponents.swift(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
Features/*/Sources/**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
Features/*/Sources/**/*.swift: SwiftUI views in Features should use @EnvironmentObject for navigation
Annotate UI code with @mainactor
Files:
Features/Comments/Sources/Comments/CommentsComponents.swift
**/*.swift
📄 CodeRabbit inference engine (AGENTS.md)
**/*.swift: Use Swift concurrency (async/await) instead of completion handlers where appropriate
Adopt Sendable where needed to ensure thread safety across concurrency boundaries
Files:
Features/Comments/Sources/Comments/CommentsComponents.swift
🧬 Code graph analysis (1)
Features/Comments/Sources/Comments/CommentsComponents.swift (2)
Features/Comments/Sources/Comments/CommentsViewModel.swift (1)
toggleCommentVisibility(165-185)Features/Feed/Sources/Feed/FeedView.swift (1)
post(271-307)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
🔇 Additional comments (2)
Features/Comments/Sources/Comments/CommentsComponents.swift (2)
166-166: LGTM! Native separator now visible to support hierarchy.The change to
.visiblealigns with the PR objectives to rely on the native separator, which automatically adjusts width based on the comment hierarchy defined by the leading insets.
315-315: Verify accessibility hint accuracy for VoiceOver users.The hint changed from "Double-tap" to "Tap", but VoiceOver users typically need to double-tap to activate elements with
.accessibilityAddTraits(.isButton)(one tap to focus, second to activate). Please confirm whether "Tap" is the intended and accurate instruction for VoiceOver users.
| .listRowInsets([.top, .bottom, .trailing], 16) | ||
| .listRowInsets([.leading], CGFloat((comment.level + 1) * 16)) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that chained .listRowInsets calls merge correctly.
Calling .listRowInsets twice with different edge sets requires verification that the custom extension merges the insets rather than having the second call override the first. If the second call overrides, the top/bottom/trailing insets would be lost.
Run the following script to locate the extension definition and verify its behaviour:
🏁 Script executed:
#!/bin/bash
# Description: Find the custom listRowInsets extension definition
# Search for the extension that takes an array of edges and a value
rg -n -A 10 'func listRowInsets.*\[.*Edge' --type swiftLength of output: 0
🏁 Script executed:
#!/bin/bash
# Description: Broader search for listRowInsets definition
# First, search for any mention of listRowInsets in the codebase
echo "=== Searching for listRowInsets keyword ==="
rg -n 'listRowInsets' --type swift
# Also check the target file to understand context
echo -e "\n=== Checking target file context ==="
fd -t f CommentsComponents.swiftLength of output: 499
🏁 Script executed:
#!/bin/bash
# Description: Search for extension definition and file context
echo "=== Searching for extension definitions with listRowInsets ==="
rg -n 'extension.*View' --type swift -A 15 | grep -A 15 'listRowInsets'
echo -e "\n=== Checking target file beginning (imports and declarations) ==="
head -50 Features/Comments/Sources/Comments/CommentsComponents.swift
echo -e "\n=== Searching extension directories ==="
fd -t f '\.swift$' | grep -i -E '(extension|helper|util)' | head -20Length of output: 3782
🏁 Script executed:
#!/bin/bash
# Description: Search for listRowInsets method definition more broadly
echo "=== Searching for listRowInsets method definition (not extension keyword) ==="
rg -n 'func listRowInsets' --type swift -B 5 -A 10
echo -e "\n=== Searching DesignSystem module for extensions ==="
fd -t f '\.swift$' DesignSystem/ | xargs grep -l 'listRowInsets' 2>/dev/null
echo -e "\n=== Searching Shared module for extensions ==="
fd -t f '\.swift$' Shared/ | xargs grep -l 'listRowInsets' 2>/dev/null
echo -e "\n=== Checking if View extension exists anywhere with any listRowInsets variant ==="
rg -n 'extension View' --type swift -A 5 | grep -B 5 'listRowInsets'Length of output: 736
Custom listRowInsets extension is missing and code will not compile.
Lines 310–311 call .listRowInsets([edges], value) with a custom signature that does not exist anywhere in the codebase. The only listRowInsets method available is the standard SwiftUI API that accepts EdgeInsets() (used at line 557).
Either implement the custom extension to support this signature, or refactor to use EdgeInsets directly:
.listRowInsets(EdgeInsets(top: 0, leading: CGFloat((comment.level + 1) * 16), bottom: 0, trailing: 16))🤖 Prompt for AI Agents
In Features/Comments/Sources/Comments/CommentsComponents.swift around lines
310–311, the calls to .listRowInsets([.top, .bottom, .trailing], 16) and
.listRowInsets([.leading], CGFloat((comment.level + 1) * 16)) use a non‑existent
custom signature and cause a compile error; replace these calls with a single
.listRowInsets(EdgeInsets(...)) call that sets top: 0, bottom: 0, trailing: 16
and leading: CGFloat((comment.level + 1) * 16) (or, if you prefer, implement a
small extension that accepts an array of Edge.Set and a value and constructs the
corresponding EdgeInsets before calling the standard .listRowInsets).
weiran
deleted the
claude/remove-comment-separator-padding-011CUkWEE8dcKN7SxEuPHG7f
branch
2 participants
Summary by CodeRabbit
Accessibility
Style