Make QuickEditor fullscreen on Wayland
ClosedPublic

Authored by davidre on Aug 26 2019, 2:23 PM.

Details

Summary

Follow up to 4177fde76d32ef0551dae11e12fa7b5cfdb6e02b. While that commit made
the QuickEditor cover all screens it wasn't over the panel anymore. This
commit ensures it is also ontop of the panel. Thanks to David Edmundson for
the code snippets.

Test Plan

QuickEditor also covers the panel

Diff Detail

Repository
R166 Spectacle
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
davidre created this revision.Aug 26 2019, 2:23 PM
Restricted Application added a project: Spectacle. · View Herald TranscriptAug 26 2019, 2:23 PM
davidre requested review of this revision.Aug 26 2019, 2:23 PM

The initial idea to use the OSD Layer didn't work because the OSDs doesn't accept keyboard focus. Panel is still high enough and can be set to accept keyboard focus.

I would like this or another fix for Applications 19.08.1 (tagging on Monday). Otherwise we would have to think about what to do/which broken version to retain. Being able to select all screens but not the panel or reverting D23346 and being able to select everything on the primary screen but nothing on the other screens.

davidre updated this revision to Diff 65143.Sep 1 2019, 5:07 PM

Really set role to Panel

davidedmundson accepted this revision.Sep 2 2019, 4:57 PM

Please see my first comment.

src/QuickEditor/QuickEditor.cpp
96

There's a fromWindow variant that's a bit nicer.

In either case, check the return value before using it on the next line, it can return a nullptr

98

This isn't sustainable long term and that needs to be remembered.

Maybe it's a sign that the plasmashell interface is too restrictive, catered around one use-case rather than being generic.
I also did a (rejected) patch that supported alwaysOnTop maybe that could be revisited.

Though first we can maybe follow xdg_toplevel.set_fullscreen() with upstream, which would be the best option.

This revision is now accepted and ready to land.Sep 2 2019, 4:57 PM
davidre added inline comments.Sep 2 2019, 5:02 PM
src/QuickEditor/QuickEditor.cpp
96

I think I tried fromWindow first but that crashed I think. What would the way forward be if it's nullptr? Just do nothing or wait till there's a surface?

davidedmundson added inline comments.Sep 2 2019, 5:05 PM
src/QuickEditor/QuickEditor.cpp
96

oh! you need to create the platform window. winId() implicitly does that.

winId();
auto surface = Surface::fromWindow(window());
if (!surface) {
    return;
}
davidre added inline comments.Sep 2 2019, 5:21 PM
src/QuickEditor/QuickEditor.cpp
96

If I need to call either way winId then is there some other reason to prefer fromWindow?

From winId just iterates through all known windows. I prefer objects to ids. But it doesn't really matter either way.

davidre updated this revision to Diff 65289.Sep 3 2019, 8:06 AM

David's comments

This revision was automatically updated to reflect the committed changes.