feat: dynamic per-tab settings window sizing#330
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe PR introduces dynamic window sizing for the Settings window based on selected tabs. A new Changes
Sequence DiagramsequenceDiagram
participant User
participant SettingsView as SettingsView
participant SettingsTab as SettingsTab
participant SettingsWindowResizer as SettingsWindowResizer
participant NSWindow as NSWindow / AppKit
User->>SettingsView: Select different tab
SettingsView->>SettingsView: Update selectedTab state
SettingsView->>SettingsTab: Resolve currentTab
SettingsTab-->>SettingsView: Return SettingsTab enum value
SettingsView->>SettingsTab: Read preferredSize property
SettingsTab-->>SettingsView: Return CGSize for tab
SettingsView->>SettingsWindowResizer: Provide size parameter
SettingsWindowResizer->>SettingsWindowResizer: updateNSView called
SettingsWindowResizer->>NSWindow: Calculate new frame (top-left anchor preserved)
SettingsWindowResizer->>NSWindow: Apply smooth animation (if visible)
SettingsWindowResizer->>NSWindow: Update minSize & maxSize constraints
NSWindow-->>User: Window resizes to preferred dimensions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
TablePro/Views/Settings/SettingsWindowResizer.swift (2)
12-13: Add explicit access control on the new type (and keepsizeimmutable).Line 12 introduces a new type without an explicit access modifier. Also,
sizedoes not need to be mutable.♻️ Proposed update
-struct SettingsWindowResizer: NSViewRepresentable { - var size: CGSize +internal struct SettingsWindowResizer: NSViewRepresentable { + let size: CGSizeAs per coding guidelines, "Always specify access control explicitly (private, internal, public) on extensions and types. Specify on the extension itself, not on individual members."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/Views/Settings/SettingsWindowResizer.swift` around lines 12 - 13, Add an explicit access control modifier to the new SettingsWindowResizer type (e.g., make the struct internal or private per the module visibility requirements) and make its size property immutable by changing var size to let size; update any callers if needed to match the chosen access level and ensure the type-level access modifier is used on the struct declaration (not on individual members).
20-31: Avoid redundant frame updates when size constraints are already satisfied.
updateNSViewcurrently re-applies frame and constraints every update pass. Add a fast-path return to avoid unnecessary AppKit work.⚡ Proposed update
func updateNSView(_ nsView: NSView, context: Context) { guard let window = nsView.window else { return } let contentSize = size let newFrameSize = window.frameRect(forContentRect: NSRect(origin: .zero, size: contentSize)).size + if window.frame.size == newFrameSize, + window.minSize == newFrameSize, + window.maxSize == newFrameSize { + return + } var frame = window.frame // Pin top-left corner frame.origin.y += frame.size.height - newFrameSize.height frame.size = newFrameSize🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@TablePro/Views/Settings/SettingsWindowResizer.swift` around lines 20 - 31, updateNSView is re-setting the window frame and min/max sizes on every call; add a fast-path that returns early when no change is required by comparing the computed newFrameSize and current window.frame.size and current constraints: if newFrameSize equals window.frame.size (or window.frameRect(forContentRect:) would result in the same outer frame) and window.minSize == newFrameSize and window.maxSize == newFrameSize then exit the method without calling window.setFrame or re-assigning min/max sizes; keep the existing top-left pinning logic but only perform it when a real size change is needed (refer to updateNSView, newFrameSize, frame, window.minSize and window.maxSize).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@TablePro/Views/Settings/SettingsWindowResizer.swift`:
- Around line 12-13: Add an explicit access control modifier to the new
SettingsWindowResizer type (e.g., make the struct internal or private per the
module visibility requirements) and make its size property immutable by changing
var size to let size; update any callers if needed to match the chosen access
level and ensure the type-level access modifier is used on the struct
declaration (not on individual members).
- Around line 20-31: updateNSView is re-setting the window frame and min/max
sizes on every call; add a fast-path that returns early when no change is
required by comparing the computed newFrameSize and current window.frame.size
and current constraints: if newFrameSize equals window.frame.size (or
window.frameRect(forContentRect:) would result in the same outer frame) and
window.minSize == newFrameSize and window.maxSize == newFrameSize then exit the
method without calling window.setFrame or re-assigning min/max sizes; keep the
existing top-left pinning logic but only perform it when a real size change is
needed (refer to updateNSView, newFrameSize, frame, window.minSize and
window.maxSize).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 22b17cc0-09ae-43aa-b2d2-5f7dc11c4199
📒 Files selected for processing (2)
TablePro/Views/Settings/SettingsView.swiftTablePro/Views/Settings/SettingsWindowResizer.swift
1 participant
Summary
SettingsWindowResizer(NSViewRepresentable) that animates the NSWindow size on tab change, keeping the top-left corner pinned (standard macOS preferences behavior)Test plan
Summary by CodeRabbit
New Features