WIP: Label inherits View and clean event handling code#450
WIP: Label inherits View and clean event handling code#450yostane wants to merge 7 commits intocompnerd:mainfrom
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 :) |
yostane
changed the title
|
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, |
compnerd
left a comment
compnerd
left a comment
There was a problem hiding this comment.
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.
Why did you move this below the class?
There was a problem hiding this comment.
Oups, I put it back in the wrong location 😁
| 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.
Was there a reason for this move?
There was a problem hiding this comment.
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. |
3 participants
Fixes: #442