Fix and docs: add hybrid mode documentation to README and DocC and fix code review issues #2
Conversation
… from hidden state The floatingHybridModeActive flag was being reset in _expand() before the guard check, which broke the floating fallback when compact() was called from .hidden state. The fix adds a resetHybridMode parameter to _expand() that defaults to true but is set to false when called from _compact(), preserving the hybrid mode flag set by the floating fallback logic. Also adds a test case to verify compact() works correctly from hidden state.
Document the new hybrid mode feature: - README: Add Hybrid Mode section with code example and floating style behavior - DocC: Add Hybrid Mode section explaining the feature and automatic fallback
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds hybrid-mode documentation and tests, refines internal expand behavior with a new Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @dima6312, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on improving the clarity and robustness of the 'Hybrid Mode' within DynamicNotchKit. It introduces detailed documentation to guide users on its implementation and behavior, particularly concerning floating style notches. Concurrently, it resolves a critical bug that prevented the hybrid mode from activating correctly when Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces documentation for the new 'hybrid mode' feature in both the README and DocC files. It also includes a crucial bug fix related to the floating style's hybrid mode behavior, where calling compact() from a hidden state was not working as expected. The fix is well-implemented and is accompanied by a new unit test to verify the corrected behavior. My review includes a suggestion to clarify some wording in the README to avoid potential user confusion. Overall, this is a good update that improves both functionality and documentation.
README.md
Outdated
|
|
||
| ### Floating Style Behavior | ||
|
|
||
| On Macs without a notch (using the `.floating` style), calling `compact()` will automatically enable hybrid mode and expand the window, showing compact indicators alongside the expanded content. This provides a consistent UX across all Mac models - your compact indicators are always visible regardless of hardware. |
There was a problem hiding this comment.
The phrase "your compact indicators are always visible regardless of hardware" could be slightly misleading, as they are not visible at all times. They appear when compact() is called, or if showCompactContentInExpandedMode is enabled while the notch is expanded. To prevent potential confusion for library users, I suggest rephrasing this to be more precise about when the indicators are shown.
| On Macs without a notch (using the `.floating` style), calling `compact()` will automatically enable hybrid mode and expand the window, showing compact indicators alongside the expanded content. This provides a consistent UX across all Mac models - your compact indicators are always visible regardless of hardware. | |
| On Macs without a notch (using the `.floating` style), calling `compact()` will automatically enable hybrid mode and expand the window, showing compact indicators alongside the expanded content. This provides a consistent UX across all Mac models, ensuring your compact indicators are displayed when requested, regardless of hardware. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Tests/DynamicNotchKitTests/DynamicNotchKitTests.swift (1)
452-478: Good test coverage for the hybrid mode fix.This test validates the critical scenario where
compact()is called directly from a hidden state, ensuring that hybrid mode is correctly activated. The assertions properly verify both the internal flag and the computed property.One minor suggestion: Consider adding a brief sleep after line 470 to allow animations to complete, similar to other tests in the file. This ensures state transitions are fully settled before assertions:
Optional: Add animation delay
// Call compact() directly from hidden state (without calling expand() first) await notch.compact() +try await Task.sleep(for: .seconds(0.5)) // Should auto-expand with hybrid mode enabled
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.mdSources/DynamicNotchKit/Documentation.docc/Documentation.mdSources/DynamicNotchKit/DynamicNotch/DynamicNotch.swiftTests/DynamicNotchKitTests/DynamicNotchKitTests.swift
🧰 Additional context used
🧬 Code graph analysis (1)
Tests/DynamicNotchKitTests/DynamicNotchKitTests.swift (2)
Sources/DynamicNotchKit/DynamicNotch/DynamicNotch.swift (2)
compact(239-241)hide(310-316)Sources/DynamicNotchKit/DynamicNotchInfo/DynamicNotchInfo.swift (2)
compact(113-120)hide(122-124)
🔇 Additional comments (4)
Sources/DynamicNotchKit/DynamicNotch/DynamicNotch.swift (2)
203-210: LGTM! Clean solution for preserving hybrid mode state.The
resetHybridModeparameter elegantly solves the issue where hybrid mode would be incorrectly reset during the compact-to-expand flow. The default value oftruemaintains backward compatibility for explicitexpand()calls, whilefalsepreserves the flag when transitioning fromcompact().The placement before the guard check (line 212) is correct - this ensures the flag is reset even when already expanded, which matches the documented behavior.
279-279: All call sites of_expand()correctly handle the newresetHybridModeparameter.Verified that all three call sites are properly configured: the public
expand()method (line 200) andDynamicNotchInfowrapper (line 107) rely on the sensible default oftrue, while the internal call from_compact()(line 279) explicitly passesfalseto preserve hybrid mode state during the compact-to-expand transition.README.md (1)
49-71: Clear and helpful documentation for the hybrid mode feature.The documentation effectively explains the hybrid mode feature with a practical code example. The "Floating Style Behavior" subsection is particularly valuable as it clarifies the automatic behavior on Macs without a notch, helping users understand why compact indicators appear in expanded mode on those devices.
Sources/DynamicNotchKit/Documentation.docc/Documentation.md (1)
17-22: Concise and consistent hybrid mode documentation.The documentation aligns well with the README and provides the essential information users need. The placement before "The Vision" section is appropriate as it introduces a key feature early in the docs.
- Clarify README wording: indicators are "displayed when requested" not "always visible" - Fix icon sizing in floating hybrid mode by adding proper safeAreaInsets - Fix rapid state transition rendering by using opacity instead of conditional rendering - Add animation delay in test for state transition reliability - Apply swiftformat 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…tions Add .fixedSize() and .layoutPriority(1) to compact leading/trailing content in floating view so SwiftUI preserves their intrinsic sizes even during rapid state changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Sources/DynamicNotchKit/DynamicNotchInfo/DynamicNotchInfo+HelperViews.swift (1)
19-19: Appropriate API surface reduction.Removing the
publicmodifier from thebodyproperty is correct—it's an implementation detail that shouldn't be exposed as public API.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
README.mdSources/DynamicNotchKit/DynamicNotch/DynamicNotch.swiftSources/DynamicNotchKit/DynamicNotchInfo/DynamicNotchInfo+HelperViews.swiftSources/DynamicNotchKit/Views/NotchView.swiftSources/DynamicNotchKit/Views/NotchlessView.swiftTests/DynamicNotchKitTests/DynamicNotchKitTests.swift
🧰 Additional context used
🧬 Code graph analysis (2)
Sources/DynamicNotchKit/DynamicNotchInfo/DynamicNotchInfo+HelperViews.swift (1)
Sources/DynamicNotchKit/Utility/BlurModifier.swift (2)
body(13-16)body(24-27)
Tests/DynamicNotchKitTests/DynamicNotchKitTests.swift (2)
Sources/DynamicNotchKit/DynamicNotch/DynamicNotch.swift (2)
compact(239-241)hide(310-316)Sources/DynamicNotchKit/DynamicNotchInfo/DynamicNotchInfo.swift (2)
compact(113-120)hide(122-124)
🔇 Additional comments (13)
Sources/DynamicNotchKit/Views/NotchView.swift (1)
196-197: LGTM! Cleaner signature with opaque parameter.The refactor from a generic constrained type parameter to an opaque
some Viewparameter simplifies the signature while maintaining identical functionality.Sources/DynamicNotchKit/Views/NotchlessView.swift (3)
71-78: Good approach to avoid SwiftUI rendering glitches.Using opacity-based visibility instead of conditional rendering helps prevent SwiftUI animation issues during rapid state transitions. The
fixedSize()andlayoutPriority(1)modifiers ensure the content maintains consistent sizing.
88-95: Consistent visibility pattern applied.The same opacity-based approach is correctly applied to the trailing content, maintaining symmetry with the leading content implementation.
100-100: Transition moved to row level for consistency.Moving the transition to the entire
compactIndicatorsRow()ensures unified animation behavior now that the views are always present with opacity control.Sources/DynamicNotchKit/DynamicNotchInfo/DynamicNotchInfo+HelperViews.swift (2)
37-37: Consistent visibility reduction.Same encapsulation improvement applied to
CompactTrailingView.
52-52: Consistent visibility reduction.Same encapsulation improvement applied to
InfoView.README.md (1)
49-71: Clear and accurate hybrid mode documentation.The documentation accurately describes the hybrid layout feature with a practical code example. Line 70's wording "displayed when requested" correctly addresses the previous review feedback about avoiding the misleading phrase "always visible."
Sources/DynamicNotchKit/DynamicNotch/DynamicNotch.swift (4)
81-81: Minor syntactic cleanup.Using
.init(EmptyView())instead ofAnyView(EmptyView())is slightly more concise while maintaining identical semantics.
105-105: Type annotation refinement.
Task<(), Never>is equivalent toTask<Void, Never>and slightly more conventional in Swift.
203-210: Core fix: conditional hybrid mode reset.The
resetHybridModeparameter elegantly solves the hybrid mode reset bug. When_expandis called from_compact(line 279), passingresetHybridMode: falsepreserves thefloatingHybridModeActiveflag, ensuring compact indicators remain visible during the hidden→compact→expanded transition on floating-style notches.The default value of
truemaintains backward compatibility for directexpand()calls.
279-279: Correct preservation of hybrid mode flag.Passing
resetHybridMode: falsehere ensures that thefloatingHybridModeActiveflag set earlier in_compact(line 277) is preserved when_expandis called, fixing the floating-style compact() behavior from hidden state.Tests/DynamicNotchKitTests/DynamicNotchKitTests.swift (2)
452-479: Excellent test coverage for the hybrid mode fix.This test comprehensively validates the core bug fix—calling
compact()from hidden state on a floating notch should expand with hybrid mode active. The assertions verify:
- State transitions correctly (hidden → expanded)
- Internal
floatingHybridModeActiveflag is set- User-facing
showCompactContentInExpandedModeproperty remains unchanged- The computed
isHybridModeEnabledreflects the correct stateThe inline comments clearly document the expected behavior.
492-492: Minor formatting consistency.Spacing adjustment aligns with Swift style conventions.
dima6312
merged commit 2cb676d
into
feature/hybrid-compact-expanded-mode
* feat: add showCompactContentInExpandedMode for hybrid notch layouts Adds support for "hybrid" layouts where compact leading/trailing content remains visible while in expanded state. This enables use cases like dictation UIs with waveform indicators alongside the notch while transcript content shows below. Changes: - Add `showCompactContentInExpandedMode` property (defaults to false) - Add init parameter for convenience - Implement symmetric layout in NotchView for hybrid expanded mode - Add comprehensive documentation for layout logic - Add test cases for notch and floating styles Also fixes: - Prevent continuation leak in hide() when closePanelTask is cancelled 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Make NSScreen extensions public Expose `hasNotch`, `notchSize`, `notchFrame`, `menubarHeight`, and `screenWithMouse` for client apps that need to detect notch presence and adjust their behavior accordingly (e.g., always using expand() instead of compact() on non-notch Macs). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Auto-enable hybrid mode in floating style for UX consistency When compact() is called in floating mode, instead of hiding the window, expand with hybrid mode enabled. This ensures compact indicators (like waveforms, status icons, cancel buttons) remain visible on non-notch Macs, providing consistent UX across all Mac hardware. Previously, non-notch Mac users would not see compact content at all since floating mode has no physical notch to flank. Now they see the same indicators as notch Mac users, displayed alongside the expanded content. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Add hybrid mode support for floating style fallback - Add internal floatingHybridModeActive flag to avoid mutating user property - Add isHybridModeEnabled computed property for unified hybrid mode check - Update compact() to auto-enable hybrid mode on floating style - Add compact indicators overlay to NotchlessView for floating panels - Fix animation when transitioning to hybrid mode from expanded state - Add test cases for hybrid mode and floating fallback behavior 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: Add compactCenterContent for floating fallback mode - Add compactCenterContent property for center UI in floating fallback - Add compactCenter case to DynamicNotchSection enum - Update NotchlessView to show indicators row with center content - Remove top inset gap when in hybrid mode - Reset floatingHybridModeActive on hide to prevent state persistence 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: smooth direct transition from expanded to compact state Remove intermediate hide step when transitioning from expanded to compact, eliminating the visual flash. Uses conversionAnimation for seamless morphing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Prevent race condition when cancelling hide task Only call deinitializeWindow() if task wasn't cancelled, since a new window may have been opened by expand()/compact() after cancellation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: Await hybrid mode animation to prevent race with hide() Changed fire-and-forget Task to awaited MainActor.run and added sleep to wait for animation completion before compact() returns. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * feat: make isHovering property public for client observation Allow clients to observe hover state changes via Combine to implement hover-based activation patterns (e.g., activate panel when mouse enters). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: comprehensive improvements for hybrid mode reliability Thread Safety & Race Conditions: - Use structured concurrency with MainActor.run instead of unstructured Tasks - Make state checks atomic in floating mode logic - Add task cancellation on all early return paths Memory Management: - Track observeScreenParameters Task and cancel in deinit - Use weak self capture to prevent retain cycles - Only reinitialize window when state is hidden Animation & State Transitions: - Extract animationDuration constant from DynamicNotchStyle - Make compact<->expanded transitions symmetric (both direct) - Reset floatingHybridModeActive even when already expanded - Move flag reset after animation completes to prevent visual glitch API Improvements: - Make floatingHybridModeActive private(set) - Optimize redundant hybrid mode animations - Add max retry count for hover-blocked hide operations View Fixes: - Constrain compact indicators row height in NotchlessView Tests: - Add comprehensive assertions for state verification - Add tests for flag resets, computed property logic, rapid transitions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Fix and docs: add hybrid mode documentation to README and DocC and fix code review issues (#2) * fix: preserve floatingHybridModeActive when compact() calls _expand() from hidden state The floatingHybridModeActive flag was being reset in _expand() before the guard check, which broke the floating fallback when compact() was called from .hidden state. The fix adds a resetHybridMode parameter to _expand() that defaults to true but is set to false when called from _compact(), preserving the hybrid mode flag set by the floating fallback logic. Also adds a test case to verify compact() works correctly from hidden state. * docs: add hybrid mode documentation to README and DocC Document the new hybrid mode feature: - README: Add Hybrid Mode section with code example and floating style behavior - DocC: Add Hybrid Mode section explaining the feature and automatic fallback * fix: improve floating hybrid mode reliability and documentation - Clarify README wording: indicators are "displayed when requested" not "always visible" - Fix icon sizing in floating hybrid mode by adding proper safeAreaInsets - Fix rapid state transition rendering by using opacity instead of conditional rendering - Add animation delay in test for state transition reliability - Apply swiftformat 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: prevent compact indicator layout compression during rapid transitions Add .fixedSize() and .layoutPriority(1) to compact leading/trailing content in floating view so SwiftUI preserves their intrinsic sizes even during rapid state changes. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> * fix: improve code quality and accessibility - Remove unused skipHide parameter from _expand() and _compact() - Add .accessibilityHidden() to opacity-hidden views for VoiceOver - Fix rapid transition rendering by always rendering compact row (use frame height instead of conditional rendering to prevent SwiftUI view identity corruption) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: improve floating mode icon layout and animation smoothness - Apply animation at VStack level for smooth container resize when hiding top row - Use explicit frame constraints for compact icons instead of fixedSize() - Disable matchedGeometryEffect in hybrid mode to prevent icon animation conflicts - Forward objectWillChange from internal DynamicNotch to DynamicNotchInfo - Reset shouldSkipHideWhenConverting when compactLeading is explicitly changed - Animate floatingHybridModeActive reset with conversionAnimation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * fix: improve code quality, fix race conditions, and refine test behavior - Fix race condition in closePanelTask that caused missing trailing icon on rapid transitions - Add explicit frame constraints for compact icons in NotchView (fixes gradient sizing in notch mode) - Update gradient test to skip compact() in floating mode (tested separately in hybrid mode tests) - Expose showCompactContentInExpandedMode parameter on DynamicNotchInfo - Remove duplicated doc comments in DynamicNotchStyle - Update comment to reflect actual purpose (removed inset references) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
Summary by cubic
Fixes a floating-style bug where hybrid mode was reset when compact() expanded from the hidden state, so compact indicators stay visible and the window auto-expands. Adds Hybrid Mode docs to README/DocC and improves floating fallback reliability by rendering indicators with opacity, adding safe-area insets, and preventing layout compression during rapid transitions; also introduces a resetHybridMode flag in _expand and a test for the hidden→compact path.
Written for commit bc600f0. Summary will update automatically on new commits.