-
-
Notifications
You must be signed in to change notification settings - Fork 57
View .onHover { Bool in } modifier #212
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
base: main
Are you sure you want to change the base?
Conversation
Added: - onHover(_ action: (Bool) -> Void) View Extension - OnHoverModifier TypeSafeView - createHoverTarget(wrapping: Widget) -> Widget to AppBackend Protocol - updateHoverTarget(_: Widget, environment: EnvironmentValues, action: (Bool) -> Void) to AppBackend Protocol - corresponding default implementations - AppKitBackend hover implementation - createHoverTarget implementation - updateHoverTarget implementation - NSCustomHoverTarget (NSView notifying about hovers) - HoverExample Fixed: - AppKitBackend - fixed reference removing for NSClickGestureRecognizer in NSCustomTapGestureTarget
…ming fixes in AppKitBackend - tapGestureRecognizer and longPressGestureRecognizer now get removed if their corresponding handler is set to nil. - Added Hoverable Widget - Added hovertarget creation & update functions - added macCalatalyst as compatible for UISlider since it supports it
…ming fixes in AppKitBackend - tapGestureRecognizer and longPressGestureRecognizer now get removed if their corresponding handler is set to nil. - Added Hoverable Widget - Added hovertarget creation & update functions - added macCalatalyst as compatible for UISlider since it supports it
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.
Thanks for the PR! This will be quite useful for making more interactive apps. I've just requested a few changes around some implementation details but overall your approach to implementing this modifier was great.
// should be impossible at the moment of implementation | ||
// keeping it to be save in case of later changes |
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.
It's safe to remove these comments, I don't think anyone will question that this is sensible (because the current guarantee that hoverChangesHandlers won't get set to nil
is external to the HoverableWidget type)
hoverTarget.width = hoverTarget.child!.width | ||
hoverTarget.height = hoverTarget.child!.height |
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.
I realise that this was copied from the tap gesture modifier, but I've just noticed that it is probably redundant (in both cases) since both modifiers call backend.setSize
just before calling the corresponding updateBlahTarget methods.
I'm happy to try that out on my end after merging though if a getting a Windows VM up is a bit annoying on your end.
} | ||
|
||
override func mouseExited(with event: NSEvent) { | ||
// Mouse exited the view's bounds |
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.
Remove this comment
// should be impossible at the moment of implementation | ||
// keeping it to be save in case of later changes |
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.
Similar to UIKitBackend, remove this comment, the code is sensible
options: options, | ||
owner: self, | ||
userInfo: nil) | ||
self.addTrackingArea(trackingArea!) |
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.
Avoid the force unwrap with an intermediate variable like you did in the didSet above.
Gtk can be pretty weird about availability information. I think the best we can do is add a hardcoded check to filter out the enum case in |
That's interesting that |
# Conflicts: # Sources/GtkCodeGen/GtkCodeGen.swift
I can’t test wether the linux compile bug is fixed because I don’t have a vm with older gtk atm. But I don’t see it in the generated code anymore. hopefully the build will now succeed on WinUIBackend the explicit frame adjustments were indeed not necessary |
The PR froze safari for a few seconds when I tried to view the new changes because there are 266 changed files 😅 I think running |
didn’t help did it? |
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.
Just two small changes to make, other than that everything looks ready to merge
Sources/GtkCodeGen/GtkCodeGen.swift
Outdated
//can cause problems with gtk versions older than 4.20.0 | ||
guard | ||
member.cIdentifier != "GTK_PAD_ACTION_DIAL", | ||
member.name != "GTK_PAD_ACTION_DIAL" |
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 this check I meant that you should check that the enumeration name is "PadActionDial" when checking the member cIdentifier. I realise that the member names are prefixed so there won't be clashes, but it's probably nice for future refactorers to have the enumeration name check there so they know which enum the rule is meant to apply to.
guard enumeration.name != "PadActionDial" || member.cIdentifier != "GTK_PAD_ACTION_DIAL" else {
return false
}
(the enum name may be GtkPadActionDial
instead of PadActionDial
, not sure)
Sources/GtkCodeGen/GtkCodeGen.swift
Outdated
@@ -224,6 +226,14 @@ struct GtkCodeGen { | |||
return false | |||
} | |||
|
|||
//can cause problems with gtk versions older than 4.20.0 |
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.
Put a space after the //
and capitalise Can
at the start of the sentence (to match the style of other comments)
added both "GtkPadActionDial" and "PadActionDial" because I couldn't find a definitive answer what its called
added both "GtkPadActionDial" and "PadActionDial" because I couldn't find a definitive answer what its called # Conflicts: # Sources/GtkCodeGen/GtkCodeGen.swift
(I somehow accidentally deleted the member.doc check, just readded it in the Amend) |
This PR introduces a new .onHover modifier to Views.
Related Issue: #211
Added in this PR:
Extension View
protocol AppBackend
Backend Support
Other changes:
HoverExample erfolgreich getestet auf:
macOS 26 (AppKitBackend, GtkBackend, UIKitBackend (macCatalyst))
iPadOS 26 (UIKitBackend)
Windows 11 (WinUI Backend)
Fedora 44 (GtkBackend)
Notes:
Gtk versions below 4.20.0 may encounter compiling problems. I don’t know why or what caused it, I only used the generate script, but on 4.18.6 on Fedora 42 I got this Error:
I checked and it should’ve been available since 4.0 https://docs.gtk.org/gtk4/enum.PadActionType.html
Maybe someone knows how to fix it and maybe it was just a local issue.
I made the weird Array-in-foreach-decision, since closed ranges didn't seem to be supported yet.
test.sh output