search

google / flutter-desktop-embedding

Experimental plugins for Flutter for Desktop
Apache License 2.0
7.1k stars 608 forks source link

Add GTK file_chooser plugin #745

Closed robert-ancell closed 4 years ago

stuartmorgan commented 4 years ago

Did you want me to review these two draft PRs, or do they need more changes first?

robert-ancell commented 4 years ago

Did you want me to review these two draft PRs, or do they need more changes first?

I haven't been able to test this yet as a recent Flutter upgrade broke my test program but this PR is basically done. The window_size plugin needs a bit more work.

robert-ancell commented 4 years ago

The existing Linux plugin has registrar->EnableInputBlockingForChannel(kChannelName) - what is the reason for this function (is it about making the dialog modal)? Do we need an equivalent here?

stuartmorgan commented 4 years ago

The existing Linux plugin has registrar->EnableInputBlockingForChannel(kChannelName) - what is the reason for this function (is it about making the dialog modal)?

IIRC the issue was that the main GLFW window would keep handling mouse input while the dialog was up, so you could accidentally trigger UI elements even though the GTK dialog acted modal.

Do we need an equivalent here?

Hopefully not :)

robert-ancell commented 4 years ago

@stuartmorgan this is ready for review. I haven't resolved my issues with the current Flutter build not working locally so I haven't yet fully tested it.

robert-ancell commented 4 years ago

The existing code uses deprecated monitor querying APIs gdk_screen_get_monitor_geometry() etc. The new API requires GTK 3.22. Can we depend on that? Ubuntu uses GTK >3.22 since 18.04 LTS. Flutter treats the deprecation warning as errors, so we'll have to disable them to make it work in newer versions.

stuartmorgan commented 4 years ago

The existing code uses deprecated monitor querying APIs gdk_screen_get_monitor_geometry() etc. The new API requires GTK 3.22. Can we depend on that? Ubuntu uses GTK >3.22 since 18.04 LTS. Flutter treats the deprecation warning as errors, so we'll have to disable them to make it work in newer versions.

I think this comment is in the wrong PR? Per my comment there, I'm fine requiring 18.04 here (and probably for Flutter on Linux in general)