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

Capture first mouse event on macOS. #842

Merged
merged 5 commits into from
May 11, 2020
Merged

Conversation

xStrom
Copy link
Member

@xStrom xStrom commented Apr 14, 2020

This PR allows druid to receive the first mouse click when the click also makes the druid window active.

When you open the macOS calculator, switch to another app, then click a calculator button it will register the click even though it wasn't active. This does not work with the druid calculator example. With this PRs change applied, the druid calculator will also receive the click event.

@xStrom xStrom added S-needs-review waits for review shell/mac concerns the macOS backend labels Apr 14, 2020
@cmyr
Copy link
Member

cmyr commented Apr 14, 2020

This is interesting; it's false by default, and I think that's reasonable. I could imagine this being an option, but I'm not totally sure we would want it to be on by default?

@xStrom
Copy link
Member Author

xStrom commented Apr 14, 2020

This is true by default for controls like NSButton. It's widely true in Apple developed macOS apps. Safari has it true for buttons, Notes has it true, Calculator has it true etc. Right now druid is the one false example I have.

I think it's completely reasonable to have it as true. What's the downside here? Some non-controller widget getting a click event?

@cmyr
Copy link
Member

cmyr commented Apr 16, 2020

The downside is clicking in a background window to gain focus, and accidentally initiating some interaction, like deselecting some selected text or applying some edit to an image in an image editor, etc.

Notes does not have this behaviour if you click in the text region; it does not change the selection. (select all in notes, focus another app, click in the selected region; the selection is unchanged).

I agree that there's an annoyance here where we can't set this to be true for specific widgets. I'm not sure what we could do about that.

In any case: my preference for false would probably be that it is least surprising. I won't die on this hill, but my gut says false is the right default. (As recognized by appkit, as well)

@xStrom
Copy link
Member Author

xStrom commented Apr 16, 2020

Okay, so there are two scenarios here that should be replicated to achieve a macOS feel.

  1. When clicking a background window's button it should work. NSButton works like this, as do a plethora of macOS apps like the calculator.
  2. When clicking a background window's edit field, the text selection should not change, although the edit field should probably get focus if I remember correctly.

Just having acceptsFirstMouse be true or false and nothing else won't achieve this goal.

We could however add a new field to MouseEvent like focused. This would be false almost always, but it will be true at minimum on macOS when it's the click that goes through acceptsFirstMouse.

This approach will allow us to modify widgets like TextBox to not change the selection with the focusing click, and this same opportunity would be there for custom widgets like a painting canvas.

@ratmice
Copy link
Collaborator

ratmice commented Apr 16, 2020

Not sure if other platforms besides macOS need to be taken into account here, but in X11
the first mouse event when clicking on a window is dependent upon whether the window manager forwards that first event to the window or focuses the window while suppressing the event,

at least this is the case when using window manager reparented windows/decorations.
that said I don't see how any problems could arise by platforms not being able to implement the hint and therefore not forwarding first mouse events when they would be allowed.

@xStrom
Copy link
Member Author

xStrom commented Apr 16, 2020

I tested Ubuntu 19.04 with the bundled calculator and text editor. It seems to behave exactly like Windows. Which is to say the first focusing mouse click goes to the app and works both as a button press and also changes text selection.

@ratmice are you seeing different behavior? What distro/WM?

@xStrom xStrom added S-waiting-on-author waits for changes from the submitter and removed S-needs-review waits for review labels Apr 16, 2020
@ratmice
Copy link
Collaborator

ratmice commented Apr 16, 2020

@xStrom Most of this I just know from having written a window manager before, by default my window manager seems to send the event by default.

I know that the windowmaker has settings for this, and tested with that with both settings.
Window focus preferences The "Do not let applications receive the click used to focus windows." near the bottom right,

With that enabled we see the weird behavior that when non-focused we still highlight buttons on MouseMove but the click used to focus doesn't register the button press.

With it disabled we get the highlight on MouseMove and the button press like expected.

Anyhow I just wanted to note that X11 is very hard to pin down any sort of feel in this regard.

@xStrom
Copy link
Member Author

xStrom commented Apr 16, 2020

Yeah I think this isn't going to be a problem if the user expects this kind of behavior. Druid matching the X11 feel will depend on what that configuration setting is for them. If the click gets sent, druid handles it and everything works. If the click doesn't get sent due to that X11 setting then that's fine too, because the user doesn't expect it to work, because it doesn't work that way with any other app on their desktop. It's simple because it's a simple boolean setting.

Windows is also simple, because it's a simple boolean that's always true.

However with macOS it's trickier because it's context/widget dependant. Thus I think the solution here is to allow druid apps to also handle the focusing click differently, however each widget wants.

@cmyr
Copy link
Member

cmyr commented Apr 17, 2020

We could however add a new field to MouseEvent like focused. This would be false almost always, but it will be true at minimum on macOS when it's the click that goes through acceptsFirstMouse.

This approach will allow us to modify widgets like TextBox to not change the selection with the focusing click, and this same opportunity would be there for custom widgets like a painting canvas.

This sounds like an elegant solution to me.

@xStrom xStrom added S-needs-review waits for review and removed S-waiting-on-author waits for changes from the submitter labels May 8, 2020
@xStrom
Copy link
Member Author

xStrom commented May 8, 2020

I pushed a new iteration of this. The information about the click being the focusing one is now made available to users. I also updated the TextBox widget to not change the selection with the focusing click.

@cmyr
Copy link
Member

cmyr commented May 8, 2020

hmm, slightly confused here; with both master and this patch, if I run the calculator and have it not be focused, a click gives it focus but is not handled.

@xStrom
Copy link
Member Author

xStrom commented May 8, 2020

That's interesting. I just re-tested and the calculator works fine, it registers the click and a new number appears on the calc display.

I'm running macOS Catalina 10.15.4.

Maybe double check that the code you have checked out matches the latest tip here and perhaps also try cargo clean.

Copy link
Member

@cmyr cmyr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, this is working for me; not sure what was going on.

My gut feeling is that this behaviour should be opt-in, not opt-out, but I'm happy enough to get this in and then see how it feels.

@xStrom xStrom merged commit 32eec9d into linebender:master May 11, 2020
@xStrom xStrom deleted the mac-first-mouse branch May 11, 2020 21:49
@xStrom xStrom removed the S-needs-review waits for review label May 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
shell/mac concerns the macOS backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants