Skip to content
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

Using SetWindowLongPtr to subclass current window to handle mouse hover/leave message #39

Merged
merged 5 commits into from
Jan 3, 2018

Conversation

chaosddp
Copy link
Collaborator

@chaosddp chaosddp commented Dec 4, 2017

@subsoap this is the way to use SetWindowLongPtr to subclass a window to handle mouse event, please have a try, when the mouse leave/enter the window, the title will be changed.
And i think it is much better to make it possible to invoke a lua callback when the mouse leave/hover. But i do not know how, i will have a try tonight.

@chaosddp chaosddp closed this Dec 4, 2017
@subsoap
Copy link
Owner

subsoap commented Dec 4, 2017

I'll test this soon.

@chaosddp chaosddp reopened this Dec 4, 2017
@subsoap
Copy link
Owner

subsoap commented Dec 4, 2017

@chaosddp
Copy link
Collaborator Author

chaosddp commented Dec 4, 2017

@subsoap now the lua callback support is completed, you can have a try, next is the name of exposed functions, i cannot find a better name, any idea?
Also, we can add other Windows messages support easily, such as sized changed event WM_SIZE as @AGulev needed.

@chaosddp chaosddp mentioned this pull request Dec 5, 2017
@subsoap
Copy link
Owner

subsoap commented Dec 5, 2017

Good job! I will test again within a day.

@dapetcu21
Copy link
Collaborator

Why expose enable_subclass_window() and disable_subclass_window() to the library user and not do them automatically at InitializeDefos() and FinalizeDefos()? The library user doesn't need to know our internals (that we're subclassing the window).

I added defos_init() and defos_final() as an interface to InitializeDefos() and FinalizeDefos() in my #43 PR. Maybe subclass there?

@subsoap
Copy link
Owner

subsoap commented Dec 29, 2017

@dapetcu21 I added you as a contributor, feel free to improve this.

@chaosddp
Copy link
Collaborator Author

chaosddp commented Jan 2, 2018

@dapetcu21 i think not all the people need this, and it may be a good idea to ask developer to determine if they need to subclass a window, or we can add a parameter to enable this on InitializeDefos?

Also my original idea is to make it a more general method that can customize other events, such as defos_events("eventname", callback), but i have no much time on this now..

@dapetcu21
Copy link
Collaborator

dapetcu21 commented Jan 2, 2018

@subsoap Thanks! What are the policies for merging code to master / reviewing PRs?

@chaosddp Is there any forseeable problem if we subclass the window anyway when the user doesn't use any of the functionality that needs subclassing? If not, then there's no benefit to not subclassing, hence no reason to expose this option (and the additional complexity it brings) to the user.

I think the API should isolate the user from platform specificities as much as we can and and adding a Windows-specific "window subclassing" concept is breaking that abstraction.

EDIT: Maybe we can subclass on-demand when the event callback is bound

@chaosddp
Copy link
Collaborator Author

chaosddp commented Jan 2, 2018

@dapetcu21 no problem if we code carefully, and i agree with you to subclass on-demand, it seems a better way.

@dapetcu21
Copy link
Collaborator

I have a bit of time today, I'll try playing a bit with this and maybe if I have time left, I'll also port this to macOS

@subsoap
Copy link
Owner

subsoap commented Jan 2, 2018

@dapetcu21 You can merge when you want to if you trust the changes. If you want a second opinion ask someone to review. Soon we will do versioned releases once Linux support is added that will be a better barrier to users being exposed to mistakes or breaking changes.

@dapetcu21
Copy link
Collaborator

@subsoap Apparently I don't have push access by being a contributor. Anyway, my changes are here: #46

@subsoap
Copy link
Owner

subsoap commented Jan 2, 2018

@dapetcu21 it says you're still waiting to respond and have not accepted collaborator access yet. I've resent invite.

@dapetcu21
Copy link
Collaborator

Oh. Didn't notice! Thanks. I'll take a look at the macOS side of things tomorrow night and it should be mergeable then.

@subsoap subsoap merged commit 7bc4174 into master Jan 3, 2018
@subsoap subsoap deleted the is_mouse_cursor_within_window branch January 9, 2018 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants