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

Add BlockingToolbar #43

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cbenhagen
Copy link

Use BlockingToolbar instead of NSToolbar to prevent maxmizing the window on double clicking a Flutter button. See macosui/macos_ui#308

@Adrian-Samoticha
Copy link
Member

Thanks. I will review this PR soon.

@Adrian-Samoticha
Copy link
Member

I don’t fully understand the behavior of the blocking toolbar. It seems like it blocks double clicks from zooming the window when the right side of the window’s toolbar is double-clicked, while still triggering the zooming when the left side of the window is clicked. It doesn’t seem to check if a Flutter widget is blocking the toolbar from being clicked.

Unless I’m misunderstanding something, I don’t think this solves the linked issue. Ideally, there would be some way to check if a Flutter widget obstructs the toolbar at the clicked position, such that the clicks only get blocked when an actual widget is being clicked.

@cbenhagen
Copy link
Author

Yes, ideally we would be able to check if a Flutter widget is in front of the toolbar and only block double clicks if this is the case. I haven't found a nice solution which allows to do this so I decided to block all double clicks to the toolbar. It is still better than zooming the window when the user double clicks a Flutter widget.

The issue you are describing is most likely due to the fact that you enable a native title in your window which will also handle the double clicks. I don't see a use case for enabling the native titles but if you do, you would most likely be better off using a completely native toolbar.

To show what is happening I added a blue background to the BlockingToolbar
Screenshot 2023-11-28 at 11 53 21
Screenshot 2023-11-28 at 11 53 17

If you see a good use case to use the native titles with a Flutter toolbar then maybe a double click blocking variant of the title could be introduced?

@Adrian-Samoticha
Copy link
Member

Right, I’d argue that it is indeed unlikely that someone would want a Flutter widget to be placed on top of the native title, so I guess that’s fine the way it is. Still, disabling the double-click feature completely would irritate people who prefer to maximize their windows by double-clicking.

Perhaps a better solution would be to add an API that enables or disables the blocking toolbar? Then, a Flutter widget could enable it for a few milliseconds when it is clicked (thereby blocking the second click), thus blocking the double-click only when the cursor is placed on top of the widget.

Do you think that would be a good idea?

@cbenhagen
Copy link
Author

I also played with this idea but currently have dropped it. It might work but every Flutter widget you place in the toolbar would need to be made aware of this. Maybe we can leave this as an improvement for later?

@Adrian-Samoticha
Copy link
Member

I think we could just offer a wrapper widget that you can wrap around your toolbar buttons, which would then handle the toolbar clicking automatically.

@cbenhagen
Copy link
Author

Can this be merged without such a wrapper widget? I think this can be improved later if anyone needs it.

@Adrian-Samoticha
Copy link
Member

I’d be willing to merge it if there was a method to enable and disable the blocking toolbar. Right now I think it’s too likely that some users may rely on the double-click-to-maximize feature in their apps to completely take it away with no way to enable it again.

Looking at your commit, I think all that’s necessary to achieve this is to add some sort of parameter of some sort to the addToolbar methods on both the Dart and the Swift side which controls whether the newly added toolbar is a BlockingToolbar or a plain NSToolbar.

If you’re unfamiliar with the way the platform channels are implemented in macos_window_utils, I’d be happy to do that myself.

@cbenhagen
Copy link
Author

I'd happily accept that offer. Please feel free to push to this branch.

@Adrian-Samoticha
Copy link
Member

How did you add the blue background to the BlockingToolbar in the screenshots you posted above? I would like to add a debug feature that allows the user to do that, but I cannot figure out how you did it.

@Adrian-Samoticha
Copy link
Member

@cbenhagen Bump, in case you missed my previous comment.

@Adrian-Samoticha Adrian-Samoticha self-assigned this Jun 10, 2024
@Adrian-Samoticha Adrian-Samoticha added bug Something isn't working enhancement New feature or request and removed bug Something isn't working labels Jun 10, 2024
@cbenhagen
Copy link
Author

Sorry, I indeed missed your comment. Unfortunately I don't remember how I did it.

@Andre-lbc
Copy link

Hi,

I need something similar to this for my app, but my use case is quite complex (it is a tab bar, like in a browser; it has fixed buttons on the sides, it scrolls horizontally, tabs can have dynamic sizes, be pinned, grouped, moved, etc). My user should be able to "drag" items on the tab bar, but this currently also moves the window (I think I've already sorted this out by also overriding mouseDownCanMoveWindow of NSView).

I will create a custom plugin using method channels and try to link size and position of flutter widgets with an equivalent "passthrough" native toolbar item in order to avoid double click to maximize and click and hold to move the window behaviors. I might try to PR a backport of it into this project if I'm successful.

It would be nice if MacosWindowUtilsConfig had a boolean to avoid calling await WindowManipulator.addToolbar(); in apply(), as my custom toolbar was being replaced with current implementation's empty toolbar (but I understand that this is very niched and already have a custom fix).

As a side note, in case you still need the background color @Adrian-Samoticha:

// ...
if itemIdentifier == NSToolbarItem.Identifier("BlockingItem") {
    let item = NSToolbarItem(itemIdentifier: itemIdentifier)
    let view = ForwardingView()
    view.flutterView = flutterView
    view.widthAnchor.constraint(lessThanOrEqualToConstant: 100000).isActive = true
    view.widthAnchor.constraint(greaterThanOrEqualToConstant: 1).isActive = true
    item.view = view
    // Add the following two lines
    view.wantsLayer = true
    view.layer?.backgroundColor = NSColor.green.cgColor
    return item
  }
  return nil
 // ...

@Adrian-Samoticha
Copy link
Member

@Andre-lbc You’re right about that being a kind of niche use case, however your toolbar with draggable items does sound useful, as many native macOS applications have that. If you can find a solution that is intuitive-enough to work with, feel free to file a PR. :)

Also, yes, I indeed still need the background color, so thank you very much. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants