-
Notifications
You must be signed in to change notification settings - Fork 71
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
WIP: Label inherits View and clean event handling code #450
base: main
Are you sure you want to change the base?
Conversation
Thanks for the patch @yostane! Does this not break the current mechanism for handling events? I think that the long term solution here is to use gesture recognizers and attach one to the button, but for the mean time, it would be nice to have something that continues to function. |
Hi you're welcome :). Ah ok I understand. What should be done is:
If that's what is expected, I will update my patch if possible. |
Yeah, I think that would be the best middle ground where we dont have the desired state, but keep the ability to detect the click. I think that follow ups for adding GestureRecognizers would be wonderful :) |
Hi @compnerd . Can you take a look at my last modifications please ? I transferred some event handling code from Control into Label to allow the developer to use the Thanks, |
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.
This does add a lot of code to Label
- and it seems most of it can be extracted into an extension. What do you think of pulling it all out into an extension and putting it into a separate file (say Label+Temporary.swift
? I do have some of the work for the gesture recognizer, we should be able to start considering adding the TapGestureRecognizer
in place instead.
} | ||
} | ||
|
||
private let SwiftLabelProc: SUBCLASSPROC = { (hWnd, uMsg, wParam, lParam, uIdSubclass, dwRefData) in |
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.
Why did you move this below the class?
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.
Oups, I put it back in the wrong location 😁
|
||
let size = self.frame.size | ||
self.hWnd_ = CreateWindowExW(0, WC_STATIC.wide, nil, DWORD(WS_CHILD), | ||
0, 0, CInt(size.width), CInt(size.height), | ||
self.hWnd, nil, GetModuleHandleW(nil), nil)! | ||
|
||
_ = SetWindowSubclass(hWnd, SwiftLabelProc, UINT_PTR(1), |
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.
Was there a reason for this move?
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.
Oups, I put it back in the wrong location 😁
I tried moving code in Label+Temporary but this seems more complex than expected because of visibility problems. The current Laabel+Temporary file contains code that did not require changes. Instead of spending more time on this temporary solution, maybe it would be better to postpone this MR and focus more on gesture recognizer. What do you think ? |
Postponing and focusing on gesture recognizer sounds like a good plan to me. @egorzhdan had previously looked at the gesture recognizer bits for menus, not sure if he has any thoughts about what exactly would be the minimum work needed to get it working. |
Yeah, I've looked into handling menu item clicks before, but I don't have a working implementation of it. |
Fixes: #442