-
Notifications
You must be signed in to change notification settings - Fork 0
feat: expose disableCompactLeading/Trailing as public API #3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Make disableCompactLeading and disableCompactTrailing properties public so developers can dynamically hide compact views on DynamicNotch instances - Fix floating mode (NotchlessView) to use conditional rendering instead of opacity, ensuring disabled views don't reserve space - Use HStack spacing for cleaner layout when views are conditionally removed No breaking changes - existing DynamicNotchInfo API continues to work unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis change exposes two internal visibility flags ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🔇 Additional comments (2)
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 enhances the 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully exposes disableCompactLeading and disableCompactTrailing as public properties, allowing for dynamic control over compact view visibility. The accompanying refactoring in NotchlessView to use conditional rendering instead of opacity is a significant improvement, as it prevents disabled views from reserving layout space and simplifies the view hierarchy. The code changes are clean and well-executed. I have a minor suggestion to improve the conciseness of the .frame modifiers.
| if !dynamicNotch.disableCompactLeading { | ||
| dynamicNotch.compactLeadingContent | ||
| .environment(\.notchSection, .compactLeading) | ||
| .frame(minWidth: compactIconSize, minHeight: compactIconSize, maxHeight: compactIconSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For improved conciseness, you can use the height parameter in the .frame modifier since minHeight and maxHeight are set to the same value (compactIconSize).
| .frame(minWidth: compactIconSize, minHeight: compactIconSize, maxHeight: compactIconSize) | |
| .frame(minWidth: compactIconSize, height: compactIconSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic is correct: When minHeight == maxHeight, using height: is cleaner.
But SwiftUI's API doesn't allow it: SwiftUI has two separate .frame() overloads:
- .frame(width:height:alignment:)
- .frame(minWidth:idealWidth:maxWidth:minHeight:idealHeight:maxHeight:alignment:)
You can't mix minWidth: (from overload 2) with height: (from overload 1) in the same call.
| if !dynamicNotch.disableCompactTrailing { | ||
| dynamicNotch.compactTrailingContent | ||
| .environment(\.notchSection, .compactTrailing) | ||
| .frame(minWidth: compactIconSize, minHeight: compactIconSize, maxHeight: compactIconSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For improved conciseness, you can use the height parameter in the .frame modifier since minHeight and maxHeight are set to the same value (compactIconSize).
| .frame(minWidth: compactIconSize, minHeight: compactIconSize, maxHeight: compactIconSize) | |
| .frame(minWidth: compactIconSize, height: compactIconSize) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gemini's suggestion was incorrect - SwiftUI doesn't have a .frame(minWidth:height:) overload. The original code is correct.
Summary
disableCompactLeadinganddisableCompactTrailingproperties public so developers can dynamically hide compact views onDynamicNotchinstancesHStack(spacing: 12)for cleaner layout when views are conditionally removedValue
DynamicNotchInfo.compactLeading = nildynamicNotch.disableCompactLeading = trueBreaking Changes
None - existing
DynamicNotchInfoAPI continues to work unchanged.Test plan
🤖 Generated with Claude Code
Summary by cubic
Expose disableCompactLeading and disableCompactTrailing as public flags to let apps hide compact indicators at runtime. Fix floating mode layout so disabled icons don’t leave gaps.
New Features
Bug Fixes
Written for commit 5a8d600. Summary will update automatically on new commits.