Add draggable New Items badge to Menu Bar Layout settings#349
Open
kylewhirl wants to merge 6 commits intostonerl:developmentfrom
Open
Add draggable New Items badge to Menu Bar Layout settings#349kylewhirl wants to merge 6 commits intostonerl:developmentfrom
kylewhirl wants to merge 6 commits intostonerl:developmentfrom
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a repositionable "New Items" badge, a shared draggable view base, persisted badge placement preferences, and updates layout bar components and defaults to support badge-driven placement of newly detected menu bar items. Changes
Sequence DiagramsequenceDiagram
participant User
participant Badge as LayoutBarNewItemsBadgeView
participant Container as LayoutBarContainer
participant Manager as MenuBarItemManager
participant Defaults as UserDefaults
User->>Badge: mouseDragged -> beginDraggingSession
Badge->>Container: NSDraggingSource willBegin (disable updates)
User->>Container: drop badge at destination
Container->>Manager: updateNewItemsPlacement(section, arrangedViews)
Manager->>Manager: compute anchor/index & persist placement
Manager->>Defaults: persistNewItemsPlacementData
Container->>Container: re-enable canSetArrangedViews, refresh arrangedViews
Container->>Badge: badge repositioned (layout updated)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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)
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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
Thaw/Resources/Localizable.xcstrings (1)
15057-15059: Tighten translator comment for accuracy.At Line 15058, the comment says this badge “displays the number of new items.” If the UI shows a static “New Items” label (not a numeric count), consider revising the comment to avoid translator confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Thaw/Resources/Localizable.xcstrings` around lines 15057 - 15059, The translator comment for the "New Items" localization key is misleading about showing a numeric count; update the comment for the "New Items" string (key "New Items") to accurately describe what the UI shows (e.g., "Static label shown on a badge" or "Label for a badge; not a numeric count") so translators know whether to translate as a static label vs. a count; edit the comment text for the "New Items" entry accordingly in Localizable.xcstrings.Thaw/MenuBar/LayoutBar/LayoutBarNewItemsBadgeView.swift (2)
14-19: Consider dynamic sizing for localized text.The badge uses a fixed size (
84 x 24) which may not accommodate longer translations of "New Items" in other languages.💡 Alternative: Calculate size from text
You could compute the badge width dynamically based on the localized string:
private lazy var titleSize: CGSize = { let attributes: [NSAttributedString.Key: Any] = [ .font: NSFont.systemFont(ofSize: 11, weight: .semibold) ] let title = String(localized: "New Items") return (title as NSString).size(withAttributes: attributes) }() private var badgeSize: CGSize { CGSize( width: titleSize.width + (Metrics.horizontalInset * 2) + 4, height: max(titleSize.height + 8, 24) ) }This would ensure the badge fits the text regardless of language. However, if all supported locales have been verified to fit within 84pt, the current approach is acceptable.
Also applies to: 56-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Thaw/MenuBar/LayoutBar/LayoutBarNewItemsBadgeView.swift` around lines 14 - 19, The badge uses a fixed Metrics.size (CGSize(width: 84, height: 24)) in LayoutBarNewItemsBadgeView which can break when the localized "New Items" string is longer; change sizing to compute the title size (e.g., add a lazy titleSize computed with the same font used in the view) and replace Metrics.size usage with a computed badgeSize that adds horizontalInset and a small padding and ensures minimum height (matching Metrics.cornerRadius or 24), then use badgeSize for layout and hit testing so the badge always fits localized text.
35-37: Potential issue:draggingImage()captures placeholder state.
bitmapImage()usescacheDisplay(in:bounds, to:rep)which renders the current view state. IfisDraggingPlaceholderistruewhendraggingImage()is called, the badge'sdraw(_:)returns early and produces a blank image.Looking at the call site in
mouseDragged(with:)(line 73),isDraggingPlaceholdershould still befalseat that point since the drag session hasn't begun yet. However, if the image is ever requested after the drag starts (e.g., for a drag update), it would be blank.This appears safe given the current flow, but consider caching the image before the drag begins or ensuring
bitmapImage()temporarily overridesisDraggingPlaceholder.Also applies to: 78-86
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Thaw/MenuBar/LayoutBar/LayoutBarNewItemsBadgeView.swift` around lines 35 - 37, The dragging image can be blank if bitmapImage() is called while isDraggingPlaceholder is true; update draggingImage() to ensure it returns a non-empty snapshot by either (a) caching the bitmap before the drag begins in mouseDragged(with:) and returning that cached image from draggingImage(), or (b) temporarily forcing isDraggingPlaceholder = false (or otherwise bypassing the early-return in draw(_:)) just for the duration of bitmapImage() so the rendered image is complete; reference the methods draggingImage(), bitmapImage(), mouseDragged(with:), and the isDraggingPlaceholder flag when making the change.Thaw/MenuBar/LayoutBar/LayoutBarArrangedView.swift (1)
74-88: Pre-existing pattern:canSetArrangedViewsnot re-enabled in drag end.The
draggingSession(_:endedAt:operation:)method does not re-enablecanSetArrangedViewson the container. This is consistent with the originalLayoutBarItemViewimplementation (per the relevant code snippets), where re-enabling happens inLayoutBarPaddingView.performDragOperation.However, if a drag session ends without calling
performDragOperation(e.g., the drag is cancelled or drops outside all padding views),canSetArrangedViewscould remainfalse, preventing subsequent cache-driven view updates.The reinsertion logic at lines 81-87 handles putting the view back, but consider also re-enabling
canSetArrangedViewsin the!hasContainerbranch to ensure the container can update in future cycles.♻️ Proposed fix to re-enable canSetArrangedViews on failed drop
func draggingSession(_: NSDraggingSession, endedAt _: NSPoint, operation _: NSDragOperation) { defer { oldContainerInfo = nil } isDraggingPlaceholder = false if !hasContainer { guard let (container, index) = oldContainerInfo else { return } container.shouldAnimateNextLayoutPass = false container.arrangedViews.insert(self, at: index) + container.canSetArrangedViews = true } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Thaw/MenuBar/LayoutBar/LayoutBarArrangedView.swift` around lines 74 - 88, The drag-end handler draggingSession(_:endedAt:operation:) can leave the original container with canSetArrangedViews == false when a drop is cancelled or falls outside padding views; update the !hasContainer branch that uses oldContainerInfo to re-enable the container by setting container.canSetArrangedViews = true (alongside container.shouldAnimateNextLayoutPass = false and container.arrangedViews.insert(self, at: index)) so the container can accept future cache-driven updates (this mirrors the behavior performed in LayoutBarPaddingView.performDragOperation).Thaw/MenuBar/LayoutBar/LayoutBarPaddingView.swift (1)
144-155: Consider the edge case when all helpers return nil in the visible section.When
arrangedViews.count > 1but allnearestItemlookups andfallbackDestinationForDraggedItemreturnnil,willMoveremainsfalseandcanSetArrangedViewsis re-enabled at line 161. This is safe, but it means the item visually moves in the UI but doesn't actually persist its new position.This can happen in the visible section since
fallbackDestinationForDraggedItemreturnsnilfor.visible. If all other arranged views are non-item views (e.g., only badges), the item would appear to drop but not move.Given the current feature scope (only one badge exists and items always exist in visible), this is unlikely to be hit in practice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Thaw/MenuBar/LayoutBar/LayoutBarPaddingView.swift` around lines 144 - 155, The current drag-drop branch can leave willMove false when arrangedViews.count > 1 but both nearestItem(toRightOf:), nearestItem(toLeftOf:) and fallbackDestinationForDraggedItem() return nil, causing the UI to show a moved item that isn't persisted; update the logic in the block handling case .item(item) (the function using arrangedViews, nearestItem(...), fallbackDestinationForDraggedItem(), willMove and move(item:to:)) to handle this edge: detect when all helper lookups return nil and in that case either compute a sensible default destination (e.g., append/prepend within visible section) or explicitly cancel the visual move and reset state before re-enabling canSetArrangedViews so the UI and model remain consistent. Ensure willMove and any state toggles (canSetArrangedViews) are updated accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Thaw` 2026-03-30 20-54-59/DistributionSummary.plist:
- Around line 1-224: Delete the committed Xcode export artifact directory
containing DistributionSummary.plist (which includes keys like Thaw, certificate
-> SHA1, team -> name "Kyle Worrall", versionNumber, embeddedBinaries, etc.),
remove the entire directory from the repo history or at least from the current
commit, and add the directory pattern to .gitignore (e.g., the Xcode
export/artifact folder) so these signing metadata files are not committed again.
In `@Thaw` 2026-03-30 20-54-59/ExportOptions.plist:
- Around line 1-14: The committed Xcode export artifact directory (contains
ExportOptions.plist with keys like teamID, destination, method) should be
removed and ignored: delete the Thaw 2026-03-30 20-54-59/ directory from the
repo (and optionally purge the file from history if sensitive), then add a
gitignore entry matching the pattern "Thaw 20*/" to .gitignore so future
timestamped Thaw export directories are not committed; commit the .gitignore
change and the removal together.
In `@Thaw.xcodeproj/project.pbxproj`:
- Line 439: The project file was accidentally updated with a personal
DEVELOPMENT_TEAM value (DEVELOPMENT_TEAM = LMMY392DLM) which will break signing
for other contributors; revert the committed changes in
Thaw.xcodeproj/project.pbxproj (remove or restore the original DEVELOPMENT_TEAM
entries) OR move the team override into a local .xcconfig (e.g., add
DEVELOPMENT_TEAM = LMMY392DLM to a developer-only xcconfig) and add that
xcconfig to .gitignore; apply this fix for all occurrences referenced (the lines
with DEVELOPMENT_TEAM at the indicated locations).
---
Nitpick comments:
In `@Thaw/MenuBar/LayoutBar/LayoutBarArrangedView.swift`:
- Around line 74-88: The drag-end handler draggingSession(_:endedAt:operation:)
can leave the original container with canSetArrangedViews == false when a drop
is cancelled or falls outside padding views; update the !hasContainer branch
that uses oldContainerInfo to re-enable the container by setting
container.canSetArrangedViews = true (alongside
container.shouldAnimateNextLayoutPass = false and
container.arrangedViews.insert(self, at: index)) so the container can accept
future cache-driven updates (this mirrors the behavior performed in
LayoutBarPaddingView.performDragOperation).
In `@Thaw/MenuBar/LayoutBar/LayoutBarNewItemsBadgeView.swift`:
- Around line 14-19: The badge uses a fixed Metrics.size (CGSize(width: 84,
height: 24)) in LayoutBarNewItemsBadgeView which can break when the localized
"New Items" string is longer; change sizing to compute the title size (e.g., add
a lazy titleSize computed with the same font used in the view) and replace
Metrics.size usage with a computed badgeSize that adds horizontalInset and a
small padding and ensures minimum height (matching Metrics.cornerRadius or 24),
then use badgeSize for layout and hit testing so the badge always fits localized
text.
- Around line 35-37: The dragging image can be blank if bitmapImage() is called
while isDraggingPlaceholder is true; update draggingImage() to ensure it returns
a non-empty snapshot by either (a) caching the bitmap before the drag begins in
mouseDragged(with:) and returning that cached image from draggingImage(), or (b)
temporarily forcing isDraggingPlaceholder = false (or otherwise bypassing the
early-return in draw(_:)) just for the duration of bitmapImage() so the rendered
image is complete; reference the methods draggingImage(), bitmapImage(),
mouseDragged(with:), and the isDraggingPlaceholder flag when making the change.
In `@Thaw/MenuBar/LayoutBar/LayoutBarPaddingView.swift`:
- Around line 144-155: The current drag-drop branch can leave willMove false
when arrangedViews.count > 1 but both nearestItem(toRightOf:),
nearestItem(toLeftOf:) and fallbackDestinationForDraggedItem() return nil,
causing the UI to show a moved item that isn't persisted; update the logic in
the block handling case .item(item) (the function using arrangedViews,
nearestItem(...), fallbackDestinationForDraggedItem(), willMove and
move(item:to:)) to handle this edge: detect when all helper lookups return nil
and in that case either compute a sensible default destination (e.g.,
append/prepend within visible section) or explicitly cancel the visual move and
reset state before re-enabling canSetArrangedViews so the UI and model remain
consistent. Ensure willMove and any state toggles (canSetArrangedViews) are
updated accordingly.
In `@Thaw/Resources/Localizable.xcstrings`:
- Around line 15057-15059: The translator comment for the "New Items"
localization key is misleading about showing a numeric count; update the comment
for the "New Items" string (key "New Items") to accurately describe what the UI
shows (e.g., "Static label shown on a badge" or "Label for a badge; not a
numeric count") so translators know whether to translate as a static label vs. a
count; edit the comment text for the "New Items" entry accordingly in
Localizable.xcstrings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4aabae44-4edf-40d2-8388-19d4185883d2
⛔ Files ignored due to path filters (1)
Thaw 2026-03-30 20-54-59/Packaging.logis excluded by!**/*.log
📒 Files selected for processing (14)
Thaw 2026-03-30 20-54-59/DistributionSummary.plistThaw 2026-03-30 20-54-59/ExportOptions.plistThaw.xcodeproj/project.pbxprojThaw/MenuBar/LayoutBar/LayoutBarArrangedView.swiftThaw/MenuBar/LayoutBar/LayoutBarContainer.swiftThaw/MenuBar/LayoutBar/LayoutBarItemView.swiftThaw/MenuBar/LayoutBar/LayoutBarNewItemsBadgeView.swiftThaw/MenuBar/LayoutBar/LayoutBarPaddingView.swiftThaw/MenuBar/LayoutBar/LayoutBarScrollView.swiftThaw/MenuBar/MenuBarItems/MenuBarItemManager.swiftThaw/Resources/Localizable.xcstringsThaw/Settings/Models/SettingsResetter.swiftThaw/Settings/SettingsPanes/MenuBarLayoutSettingsPane.swiftThaw/Utilities/Defaults.swift
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Thaw/MenuBar/LayoutBar/LayoutBarPaddingView.swift`:
- Around line 156-166: The drop path for .item in the draggingSource.kind branch
can miss a valid fallback when a section only has the New Items badge because
nearestItem(toRightOf:) and nearestItem(toLeftOf:) return nil and
fallbackDestinationForDraggedItem() reads itemCache.managedItems (which omits
badge-only controls); update this branch in the block handling .item (and the
analogous block at lines 264-278) to reuse the same live control-item lookup
used in the single-item branch (the arrangedViews-based lookup) instead of
consulting itemCache.managedItems—i.e., if both nearestItem(...) calls return
nil, check arrangedViews.count and use the live arrangedViews/control-items
logic from the single-item path to resolve a destination and then call
move(item:to:).
In `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift`:
- Around line 481-483: The current anchor lookup uses itemCache (see
sectionName(for:), newItemsPlacement.sectionKey, anchorIdentifier and the
anchorItem search) which can return logically cached items that are temporarily
shown; change the lookup to resolve anchors against the live/visible items
produced during the current cache pass (the in-memory list used to compute
layout) instead of itemCache, or skip using an anchor when that anchor item has
an active temporary-show context; ensure the code that calls move() receives an
anchor derived from the live items (or nil) so move() uses physical bounds for
placement rather than cached logical positions.
- Around line 394-396: The badge anchor persistence is using the changing
uniqueIdentifier (checked in the if with sectionName(for:
newItemsPlacement.sectionKey), newItemsPlacement.anchorIdentifier, and
itemIdentifiers.firstIndex(of: anchorIdentifier)), so anchors placed next to
dynamic-title items are lost when the title (and identifier) changes; update the
lookup to use the same stable-key fallback used by the section-order restore
logic (i.e., detect dynamic-title items and fall back from uniqueIdentifier to
the stable key used for section ordering) so that when resolving
anchorIdentifier to an index you first try the exact match on
newItemsPlacement.anchorIdentifier and, if not found and the item is a
dynamic-title entry, try the stable fallback key before defaulting to
.sectionDefault; apply the same change to the analogous block around lines
441–451 that handles anchor persistence.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 977383bc-0c71-4006-822a-4018684739a6
📒 Files selected for processing (7)
Thaw.xcodeproj/project.pbxprojThaw/MenuBar/LayoutBar/LayoutBarArrangedView.swiftThaw/MenuBar/LayoutBar/LayoutBarContainer.swiftThaw/MenuBar/LayoutBar/LayoutBarPaddingView.swiftThaw/MenuBar/MenuBarItems/MenuBarItemManager.swiftThaw/Settings/Models/SettingsResetter.swiftThaw/Utilities/Defaults.swift
💤 Files with no reviewable changes (1)
- Thaw.xcodeproj/project.pbxproj
🚧 Files skipped from review as they are similar to previous changes (3)
- Thaw/Settings/Models/SettingsResetter.swift
- Thaw/Utilities/Defaults.swift
- Thaw/MenuBar/LayoutBar/LayoutBarArrangedView.swift
kylewhirl
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift (1)
536-577:⚠️ Potential issue | 🟠 MajorDynamic-title anchors still resolve to
nil.Line 539 stores a namespace-only anchor for dynamic items, but Lines 553-555 and 571-573 immediately return when that same stable key comes back in. As a result,
resolvedNewItemsAnchorItem(...)never finds the live anchor again, so new-item routing next to dynamic-title entries still falls back to.sectionDefault.💡 Minimal fix
private func resolvedNewItemsAnchorIndex( for anchorIdentifier: String, in itemIdentifiers: [String] ) -> Int? { if let exactMatch = itemIdentifiers.firstIndex(of: anchorIdentifier) { return exactMatch } let stableIdentifier = stableNewItemsAnchorIdentifier(from: anchorIdentifier) - guard stableIdentifier != anchorIdentifier else { - return nil - } - return itemIdentifiers.firstIndex { identifier in stableNewItemsAnchorIdentifier(from: identifier) == stableIdentifier } } private func resolvedNewItemsAnchorItem( for anchorIdentifier: String, in items: [MenuBarItem] ) -> MenuBarItem? { if let exactMatch = items.first(where: { $0.uniqueIdentifier == anchorIdentifier }) { return exactMatch } let stableIdentifier = stableNewItemsAnchorIdentifier(from: anchorIdentifier) - guard stableIdentifier != anchorIdentifier else { - return nil - } - return items.first { item in persistedNewItemsAnchorIdentifier(for: item) == stableIdentifier } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift` around lines 536 - 577, The functions resolvedNewItemsAnchorIndex(...) and resolvedNewItemsAnchorItem(...) bail out early when stableNewItemsAnchorIdentifier(from: anchorIdentifier) == anchorIdentifier, preventing namespace-only dynamic anchors from matching their persisted counterparts; remove the guard/early-return so the subsequent search (itemIdentifiers.firstIndex { stableNewItemsAnchorIdentifier(from: identifier) == stableIdentifier } and items.first { persistedNewItemsAnchorIdentifier(for: item) == stableIdentifier }) still runs even when stableIdentifier equals the original anchorIdentifier, leaving the rest of the matching logic (using stableNewItemsAnchorIdentifier, persistedNewItemsAnchorIdentifier, and DynamicItemOverrides.isDynamic) intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift`:
- Around line 183-185: The persisted newItemsPlacement preference (backed by
NewItemsPlacement / newItemsPlacement) is not cleared by
resetLayoutToFreshState; update resetLayoutToFreshState to remove or reset the
Defaults entry used to persist newItemsPlacement (and any other new Defaults
keys introduced in the same change) so a layout reset truly returns to fresh
state. Locate the persistence key(s) in Defaults (the key used by
NewItemsPlacement/newItemsPlacement) and call Defaults.removeObject(forKey:
<thatKey>) or set the stored value back to NewItemsPlacement.defaultValue inside
resetLayoutToFreshState (also ensure you do the same for the additional keys
noted around lines 249–293).
---
Duplicate comments:
In `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift`:
- Around line 536-577: The functions resolvedNewItemsAnchorIndex(...) and
resolvedNewItemsAnchorItem(...) bail out early when
stableNewItemsAnchorIdentifier(from: anchorIdentifier) == anchorIdentifier,
preventing namespace-only dynamic anchors from matching their persisted
counterparts; remove the guard/early-return so the subsequent search
(itemIdentifiers.firstIndex { stableNewItemsAnchorIdentifier(from: identifier)
== stableIdentifier } and items.first { persistedNewItemsAnchorIdentifier(for:
item) == stableIdentifier }) still runs even when stableIdentifier equals the
original anchorIdentifier, leaving the rest of the matching logic (using
stableNewItemsAnchorIdentifier, persistedNewItemsAnchorIdentifier, and
DynamicItemOverrides.isDynamic) intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3fec5e70-5782-4e38-9c6a-268907933964
📒 Files selected for processing (2)
Thaw/MenuBar/LayoutBar/LayoutBarPaddingView.swiftThaw/MenuBar/MenuBarItems/MenuBarItemManager.swift
Comment on lines
+183
to
185
| /// Placement preference for newly detected menu bar items. | ||
| @Published private(set) var newItemsPlacement = NewItemsPlacement.defaultValue | ||
|
|
Contributor
There was a problem hiding this comment.
Reset Layout won't clear this new persisted preference.
This adds another layout preference under Defaults, but the later resetLayoutToFreshState() cleanup still only clears the older layout keys. After a “fresh-install state” reset, newly detected items will keep routing to the old badge location until the user moves the badge again.
Also applies to: 249-293
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Thaw/MenuBar/MenuBarItems/MenuBarItemManager.swift` around lines 183 - 185,
The persisted newItemsPlacement preference (backed by NewItemsPlacement /
newItemsPlacement) is not cleared by resetLayoutToFreshState; update
resetLayoutToFreshState to remove or reset the Defaults entry used to persist
newItemsPlacement (and any other new Defaults keys introduced in the same
change) so a layout reset truly returns to fresh state. Locate the persistence
key(s) in Defaults (the key used by NewItemsPlacement/newItemsPlacement) and
call Defaults.removeObject(forKey: <thatKey>) or set the stored value back to
NewItemsPlacement.defaultValue inside resetLayoutToFreshState (also ensure you
do the same for the additional keys noted around lines 249–293).
Collaborator
|
The linter check is failing which is blocking the build from running. Could you fix the linter errors before we take a look at this? |
|
Author
Should be fixed now, there’s still a minor visual accessibility issue between light and dark mode I need to fix but general function works |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds a draggable New Items badge to the Menu Bar Layout settings so users can choose where newly detected menu bar items should land. The selected destination is persisted and used by the menu bar item manager when routing new items into the Visible, Hidden, or Always-Hidden sections.
PR Type
• [] Bugfix
• [ ] CI/CD related changes
• [ ] Code style update (formatting, renaming)
• [ ] Documentation
• [x] Feature
• [ ] i18n/l10n (Translation/Localization)
• [x] Refactor
• [ ] Performance improvement
• [ ] Test addition or update
• [ ] Other (please describe):
Does this PR introduce a breaking change?
• [ ] Yes
• [x] No
If yes, please describe the impact and migration path:
What is the current behavior?
New menu bar items are routed by hardcoded behavior instead of a user-controlled layout setting. They effectively default into always-hidden when that section is enabled, or hidden when it is not, and there is no layout UI affordance for changing that destination.
Issue Number: N/A
What is the new behavior?
The Menu Bar Layout tab now shows a draggable New Items badge inside the layout UI, similar to Bartender behavior. Users can move that badge between Visible, Hidden, and Always-Hidden when the always-hidden section is enabled. The chosen location is persisted and used for future newly detected menu bar items.
PR Checklist
• [ x] I’ve built and run the app locally
• [ x] I’ve checked for console errors or crashes
• [x ] I've run relevant tests and they pass
• [ ] I've added or updated tests (if applicable)
• [ ] I've updated documentation as needed
• [ n/a] I've verified localized strings work correctly (if i18n/l10n changes)
Notes for reviewers
Please focus on:
• Drag/drop behavior for the New Items badge across sections
• Persistence and fallback behavior when Always-Hidden is disabled
• New-item routing in MenuBarItemManager to confirm it matches the badge location
Summary by CodeRabbit
New Features
Settings & Preferences
Localization