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

wayland: Implement make_center #1942

Merged
merged 9 commits into from
Jul 12, 2024
Merged

wayland: Implement make_center #1942

merged 9 commits into from
Jul 12, 2024

Conversation

leolost2605
Copy link
Member

@leolost2605 leolost2605 commented May 30, 2024

Add a make_center method. These surfaces are always centered and can be used for e.g. shortcut overlay, onboarding, initial setup, various dialogs (like polkit, etc.).

I plan to add a make_modal method here as well that acts like #1879

Regarding naming stuff we can of course change it to anything we want. I just used the widget interface because I thought it was meant for that before reading @tintou's comment about it. If somebody wants to call it something different and has a good name for it I'll be happy to change it :)

Fixes #1928 (IMHO it makes more sense to do it like this because a lot of stuff from the panel interface doesn't apply on centered "panels" like hide modes, barriers, struts, etc.)

@leolost2605
Copy link
Member Author

E.g. used with polkit agent: elementary/pantheon-agent-polkit#81

@tintou
Copy link
Member

tintou commented May 30, 2024

Yeah widget is meant for Desktop widgets (such as Nimbus)

@leolost2605
Copy link
Member Author

Any alternative naming suggestions? Dialog? Centered? Or should I use the extended behavior?

@tintou
Copy link
Member

tintou commented May 30, 2024

I guess that it fits the extended behavior, maybe we should have one function for centering and another for "focusing" with the rest of the desktop dimmed ? Or should we have everything in one as "ShellModal" ?

@tintou
Copy link
Member

tintou commented May 30, 2024

I believe that @danirabbit want to give some input on if we have different use cases here

@leolost2605 leolost2605 changed the title wayland: Implement widgets wayland: Implement system dialogs May 30, 2024
@leolost2605
Copy link
Member Author

leolost2605 commented May 30, 2024

Ooops I saw these comments to late, I renamed it to/introduced a system dialogs interface but let's wait for @danirabbit

@danirabbit
Copy link
Member

The use cases I can think of are:

  • Polkit
  • Onboarding
  • Installer + Initial Setup
  • session dialog

I think the only one of those where some kind of modal styling might be weird is Onboarding because it can launch system settings for expanded options for example

@leolost2605
Copy link
Member Author

Ok so I guess we should keep modal optional since it won't be too hard anyways. Would you prefer it being in the extended behavior though? IMHO having a separate interface for this kinda makes sense to keep the methods more closely related but idk

@tintou
Copy link
Member

tintou commented May 31, 2024

The "extended behavior" interface is exactly made for such cases 😉

@leolost2605
Copy link
Member Author

Ok I moved it to extended behavior and introduced a make_centered there. I would leave the modal part to another PR

@danirabbit
Copy link
Member

@leolost2605 whats the best way to test this? I tried installing this branch and the polkit agent branch but I didn't notice any difference 🤔

@leolost2605
Copy link
Member Author

leolost2605 commented Jun 22, 2024

@danirabbit since the modal behavior and dimming is left to another PR the most visible thing would probably be that it always stays in the middle and also above other windows. In theory it shouldn't be movable (e.g. via shortcuts or super + drag) but I think it has a big windowhandle on it so it manually requests moves which are still honored.

@danirabbit
Copy link
Member

@leolost2605 okay so I installed this branch and also elementary/pantheon-agent-polkit#81 and logged out and back in and I can't confirm that polkit dialogs are appearing in the center of the display, and they aren't staying always on top either, but they do seem to appear on every workspace

@leolost2605
Copy link
Member Author

Then there's definitely something wrong here, I'll take another look at it :)

@leolost2605
Copy link
Member Author

Ok so turns out that 1. I forgot to set a quark and 2. I completely forgot you have to add the polkit agent to be launched as a trusted client via the dconf key /io/elementary/wm/desktop/trusted-clients as /usr/libexec/policykit-1-pantheon/io.elementary.desktop.agent-polkit

@danirabbit
Copy link
Member

Ahhh, maybe instead of (or in addition to) a dconf key we should check if the id is prefixed with io.elementary? Since a dconf key can't be updated so there would be no way to add new trusted clients to an existing install

@leolost2605
Copy link
Member Author

Maybe the name trusted client is a bit misleading but this actually doesn't only check whether it's allowed but actually launches it so some explicit setting is pretty much a must (we don't want to launch all elementary apps at galas start). I didn't actually consider that we can't update dconf keys (oops) but maybe we can do postinstalls? Or alternatively don't use dconf but hardcode (and then allow adding customs ones/disabling hardcoded ones)?

@danirabbit
Copy link
Member

@leolost2605 yeah dconf is considered user configuration so if we want like admin/maintainer configuration probably you want a keyfile in /etc

@leolost2605
Copy link
Member Author

@danirabbit would something like #1956 work?

@leolost2605 leolost2605 changed the title wayland: Implement system dialogs wayland: Implement make_center Jul 7, 2024
@danirabbit
Copy link
Member

@tintou does this look okay to you? :)

Copy link
Member

@tintou tintou left a comment

Choose a reason for hiding this comment

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

looks good

@danirabbit danirabbit merged commit f8346f0 into master Jul 12, 2024
2 of 4 checks passed
@danirabbit danirabbit deleted the leolost/widget branch July 12, 2024 16:35
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.

Add CENTER anchor to shell API
3 participants