experiment with Animation by AryanRogye · Pull Request #48 · AryanRogye/ComfyNotch

AryanRogye · 2025-08-05T23:09:31Z

Fixing Issues related to #49 and #50

…, but it takes out alot of features that I have to add back in

…a new Space Manager, also gonna keep track with a TASKS.md, with references and whatnot

- Re-added fallback notch functionality - Updated NotchSpaceManager to prevent space manager from blocking notch display - Fixed notch re-alignment issues - Added onTapGesture to open the notch - Temporarily removed context menu (will re-add after removing touch controls) - Fixed widget spacing logic in ComfyNotchStyleMusicWidget

…meNotchView renders - Correct widget spacing by aligning TopNotchView properly - Prevent eventTap crashes when clicking with no active panel (safer guards) - Remove redundant nested HStacks in HomeNotchView to avoid extra renders - Center album art in ComfyNotchStyleMusicWidget - Internal file cleanup

…ing shows unnecessary fadeaway aniations - when closing would close then readjust the notch if music was playing, but now we flow into it - Thinking about new logic for the whole animation, where I wouldnt have to think much about it

Selected display changes are now staged until clicking Save. - Changes apply instantly without needing to restart the app - Notch height calculation now also considers menu bar height - Fixed hover-off glitch with the notch

Copilot

Pull Request Overview

This PR implements a major animation system refactor for ComfyNotch, transitioning from the old ScrollHandler to a new ScrollManager architecture. The changes introduce improved animation handling, better space management with CGS (Core Graphics Services) integration, and various UI refinements.

Replaced ScrollHandler with new ScrollManager for better animation control and notch size management
Added ComfyNotchSpaceManager using CGS APIs for advanced window space handling
Refactored ComfyNotchView with improved hover detection and animation logic

Reviewed Changes

Copilot reviewed 39 out of 41 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
ComfyNotch/Handlers/ScrollManager.swift	New animation manager replacing ScrollHandler with improved size/animation control
ComfyNotch/Managers/ComfyNotchSpaceManager.swift	New CGS-based space manager for advanced window management
ComfyNotch/Main/ComfyNotchView.swift	Major refactor with new ViewModel pattern and improved hover/animation logic
ComfyNotch/Managers/UIManager.swift	Updated to integrate with new space manager and animation system
Various widget/view files	Updated references from ScrollHandler to ScrollManager

Copilot · 2025-08-11T00:31:05Z

ComfyNotch/Handlers/ScrollManager.swift

+            print("Notch Width: \(width)")
+            return width
+        }
+
+        // Fallback
+        print("USING FALLBACK")


Use debugLog() instead of print() for consistency with the project's logging system. This will respect the DEBUG flag and logging configuration.

Suggested change

print("Notch Width: \(width)")

return width

}

// Fallback

print("USING FALLBACK")

debugLog("Notch Width: \(width)")

return width

}

// Fallback

debugLog("USING FALLBACK")

ComfyNotch/Handlers/ScrollManager.swift

ComfyNotch/Managers/Panel/PanelStores.swift

Copilot · 2025-08-11T00:31:06Z

ComfyNotch/Handlers/ScrollManager.swift

+import Combine
+
+enum ScrollController {
+    case old


The ScrollController enum is defined but only used to check between 'old' and 'new' implementations. Since this appears to be experimental code, consider removing the unused 'old' case or adding documentation about the migration strategy.

Suggested change

case old

Plan to remove soon, is a nice way to switch from old logic to new for now

Copilot · 2025-08-11T00:31:06Z

ComfyNotch/Main/ComfyNotchView.swift

+            if uiManager.panelState == .open && !isHovering {
+                // TODO: A bit buggy, needs to be fixed,
+                viewModel.handleScrollUp(translation: 51, phase: .ended)


The TODO comment indicates known buggy behavior in hover detection logic. Consider creating a proper issue or fixing the implementation before merging.

Suggested change

if uiManager.panelState == .open && !isHovering {

// TODO: A bit buggy, needs to be fixed,

viewModel.handleScrollUp(translation: 51, phase: .ended)

// Only trigger scroll up if panel is open, not hovering, and not interacting (e.g., not dragging)

let shouldTriggerScrollUp = uiManager.panelState == .open && !isHovering && !viewModel.isDragging

if shouldTriggerScrollUp {

let hoverOffTranslation: CGFloat = 51 // Use a named constant for clarity

viewModel.handleScrollUp(translation: hoverOffTranslation, phase: .ended)

- Cleaned up print statements - Fixed crash in display settings by removing force unwrapping - Resolved timing issue with notch visibility when closing - Adjusted quickAccessWidgetDistanceFromTop from 4 to 0 - Ensured Open Notch Content Dimension values are correct: - Set TopNotchView spacing to 0 when open, giving full control to the setting - Limited quickAccessWidgetDistanceFromTop max to maxNotchHeight

- Removed touch settings and replaced them with a context menu for better functionality (right-click now supports more actions) - UI tests are likely broken due to this change, will fix them soon

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Added back a new HoverView, that is much more reliable and m

Removed lots of unused code - Renamed Hover Album Target from Album Image Only to Album Image - Cleaned alot of AppDelegate replaced with AppCoordinator that handles all the logic - Added New Window Coordinator to handle all the window logic - This made sure that we can get rid of WindowGroup {} in the main App file and have more control over the windows - Fixed bug in QR Code For Filetray where it wasnt catching SettingsModel at runtime - SettingsCoordinator to handle the settings window logic, this uses the WindowCoordinator - Made sure everything flows through the SettingsCoordinator to open the settings window - Main File is much cleaner now, added new destroyViewWindow function to close the window of the assigned view - New Debug File, just moved things over from the main app file - Moved Files around to make more sense

Fixing weird bug where the settings page wouldnt be focused when opened - Now we have a number of tries we can have the window be focused - Removed unused code

Moves Files around to make more sense - Fixed weird Metal Animations description, now is more clear and not in a awkward place - Reworked full Settings Container and Settings Sections, now is using more apple native things

- License Tab wouldnt let us scroll all the way to the bottom, so I fixed that

Fixed bug in WindowCoordinator where if we opened the settings window the first time, it wouldnt trigger the onOpen - Cleaning up ScrollUp - I thought threads were started and never stopped, but turns out Xcode is just tweaking

AryanRogye · 2025-09-04T17:13:39Z

ComfyNotch/Widgets/ExpandedWidgets/MusicPlayerWidget/ComfyNotchStyleMusicWidget.swift

                                Rectangle()
                                    .fill(Color(nsColor: model.nowPlayingInfo.dominantColor))
-                                    .frame(width: max(CGFloat(effectivePosition / max(model.nowPlayingInfo.durationSeconds,1)) * geometry.size.width, 0), height: 4)
+                                    .frame(width: min(max(CGFloat(effectivePosition / max(model.nowPlayingInfo.durationSeconds,1)) * geometry.size.width, 0), geometry.size.width), height: 4)


This is the fix for the slider extending past the allowed space issue from #49

… all places can now use that logic

Copilot

Pull Request Overview

This PR introduces a significant refactoring to modernize the animation system by migrating from an older scroll-based approach to a new layout manager. The changes focus on consolidating widget layout management and improving the user interface consistency across settings panels.

Migration from ScrollHandler to a new ScrollManager for better animation control
Major refactoring of settings panels to use native SwiftUI GroupBox components with consistent padding and divider styling
Removal of legacy touch action controls and modernization of UI architecture with new coordinator patterns

Reviewed Changes

Copilot reviewed 77 out of 121 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
ComfyNotch/Views/Notch/ComfyNotchView.swift	New comprehensive notch view implementation with modern state management
ComfyNotch/Core/Handlers/ScrollManager.swift	New scroll management system replacing legacy ScrollHandler
ComfyNotch/Views/Settings/Components/ComfySettingsContainer.swift	Simplified settings container using native GroupBox styling
Multiple settings files	Consistent padding and divider styling updates across all settings panels
ComfyNotch/Core/Models/SettingsModel.swift	Removal of touch action properties and addition of proximity settings

Comments suppressed due to low confidence (2)

ComfyNotch/Views/Notch/ComfyNotchView.swift:1

"Impliment" should be "Implement".

import AppKit

ComfyNotch/Widgets/CompactWidgets/CompactAlbumWidget.swift:85

There are two onChange modifiers listening to the same property notchStateManager.hoverHandler.scaleHoverOverLeftItems (lines 69-80 and 81-85). This will cause both handlers to execute, potentially creating conflicting animations or unexpected behavior. Consolidate these into a single onChange handler.

        .onChange(of: notchStateManager.hoverHandler.scaleHoverOverLeftItems) { _, value in
            withAnimation(.interpolatingSpring(stiffness: 180, damping: 20)) {
                scale = value ? 1.3 : 1.0
            }
        }

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-09-04T20:11:34Z

ComfyNotch/Views/Shapes/NotchShape.swift

        var path = Path()

-        // Start at top-left corner
+        // MARK: - Top Left Corner


[nitpick] Consider making the comment style consistent. Some comments use // MARK: - while others don't follow this pattern. Use consistent MARK comments throughout the shape definition.

Copilot · 2025-09-04T20:11:35Z

ComfyNotch/Widgets/ExpandedWidgets/MusicPlayerWidget/ComfyNotchStyleMusicWidget.swift

                                Rectangle()
                                    .fill(Color(nsColor: model.nowPlayingInfo.dominantColor))
-                                    .frame(width: max(CGFloat(effectivePosition / max(model.nowPlayingInfo.durationSeconds,1)) * geometry.size.width, 0), height: 4)
+                                    .frame(width: min(max(CGFloat(effectivePosition / max(model.nowPlayingInfo.durationSeconds,1)) * geometry.size.width, 0), geometry.size.width), height: 4)


This complex nested calculation should be extracted into a computed property or helper function for better readability. Consider creating a progressBarWidth computed property that takes effectivePosition, durationSeconds, and geometry.size.width as parameters.

Copilot · 2025-09-04T20:11:35Z

ComfyNotch/Views/Settings/Tabs/NotchGeneralTab/OpenNotchContentDimensionsView.swift

            ComfySlider(
                value: $v.topSpacing,
-                in: 0...100,
+                in: 0...Int(ScrollManager.shared.getNotchHeight()),


ScrollManager.shared.getNotchHeight() is being called in the slider range definition which could be computed multiple times during UI updates. Consider storing this value in a computed property or state variable to avoid repeated calculations.

crazy experiment with the Scrolling Logic and Animation, THIS IS SICK…

0410b70

…, but it takes out alot of features that I have to add back in

AryanRogye requested a review from Copilot August 5, 2025 23:18

This comment was marked as outdated.

Sign in to view

AryanRogye added 3 commits August 5, 2025 23:29

added back proximity logic

05bda50

made the notch shape much better, also added the re_align_notch back

15f0df3

Added back re_align notch, added back the live activities HUD, added …

2ea1cbb

…a new Space Manager, also gonna keep track with a TASKS.md, with references and whatnot

AryanRogye requested a review from Copilot August 9, 2025 00:28

This comment was marked as outdated.

Sign in to view

AryanRogye added 4 commits August 9, 2025 00:23

Fix display settings handling

973a4e8

Selected display changes are now staged until clicking Save. - Changes apply instantly without needing to restart the app - Notch height calculation now also considers menu bar height - Fixed hover-off glitch with the notch

AryanRogye requested a review from Copilot August 11, 2025 00:30

Copilot AI reviewed Aug 11, 2025

View reviewed changes

AryanRogye requested a review from Copilot August 11, 2025 19:19

Replaced touch settings with context menu

da3acd4

- Removed touch settings and replaced them with a context menu for better functionality (right-click now supports more actions) - UI tests are likely broken due to this change, will fix them soon

Copilot AI reviewed Aug 11, 2025

View reviewed changes

AryanRogye requested a review from Copilot August 11, 2025 21:23

This comment was marked as outdated.

Sign in to view

Added back a new HoverView, that is much more reliable and m

f674691

Added back a new HoverView, that is much more reliable and m

AryanRogye requested a review from Copilot August 24, 2025 05:57

This comment was marked as outdated.

Sign in to view

AryanRogye added 2 commits August 24, 2025 02:16

syncing changes

fceeca8

AryanRogye requested a review from Copilot August 24, 2025 07:21

This comment was marked as outdated.

Sign in to view

AryanRogye added 2 commits August 24, 2025 15:50

added new Proximity Settings

97e13dc

fixed page inside the tracker

789d0d2

AryanRogye requested a review from Copilot August 25, 2025 00:17

This comment was marked as outdated.

Sign in to view

AryanRogye added 4 commits August 25, 2025 12:04

Fixing weird bug where the settings page wouldnt be focused when opened

72b82f8

Fixing weird bug where the settings page wouldnt be focused when opened - Now we have a number of tries we can have the window be focused - Removed unused code

Moves Files around to make more sense

ebbe428

Moves Files around to make more sense - Fixed weird Metal Animations description, now is more clear and not in a awkward place - Reworked full Settings Container and Settings Sections, now is using more apple native things

License Tab wouldnt let us scroll all the way to the bottom, so I fixed

6c23857

- License Tab wouldnt let us scroll all the way to the bottom, so I fixed that

syncing commits

0272ce0

AryanRogye requested a review from Copilot August 26, 2025 19:46

This comment was marked as outdated.

Sign in to view

AryanRogye requested a review from Copilot August 26, 2025 20:17

This comment was marked as outdated.

Sign in to view

AryanRogye added 2 commits August 26, 2025 15:19

fixing tests

e2b3215

sycing changes

05b6e5f

AryanRogye mentioned this pull request Aug 31, 2025

Made comfynotch wider than screen, can't access settings? #50

Closed

fixig a major weird clipping bug

c26a28c

AryanRogye commented Sep 4, 2025

View reviewed changes

AryanRogye added 4 commits September 4, 2025 12:32

moved scrolling logic into a singleton manager, the reason is so that…

bb848af

… all places can now use that logic

kinda hacky but found a weird fix for the album cover opeing the notch

ff97a6d

more controlled animation system

f7c5e05

clearing out any bugs, I added some sketchy things kinda ass but its ok

d10384e

AryanRogye requested a review from Copilot September 4, 2025 20:09

Copilot AI reviewed Sep 4, 2025

View reviewed changes

AryanRogye and others added 2 commits September 22, 2025 21:50

local testing

be7627a

Sync over small things

0072fb0

AryanRogye mentioned this pull request Feb 5, 2026

Not opening #51

Closed

-            if uiManager.panelState == .open && !isHovering {
-                // TODO: A bit buggy, needs to be fixed,
-                viewModel.handleScrollUp(translation: 51, phase: .ended)
+            // Only trigger scroll up if panel is open, not hovering, and not interacting (e.g., not dragging)
+            let shouldTriggerScrollUp = uiManager.panelState == .open && !isHovering && !viewModel.isDragging
+            if shouldTriggerScrollUp {
+                let hoverOffTranslation: CGFloat = 51 // Use a named constant for clarity
+                viewModel.handleScrollUp(translation: hoverOffTranslation, phase: .ended)

Conversation

AryanRogye commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

AryanRogye Aug 26, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Aug 11, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

AryanRogye Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Sep 4, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

AryanRogye commented Aug 5, 2025 •

edited

Loading