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

Implement Decorate/Undecorate with smart mode selection #1730

Closed
wants to merge 5 commits into from

Conversation

xi
Copy link
Contributor

@xi xi commented Apr 19, 2024

Yet another approach for #1728.

This one is basically the <action name="SetServerSideDecorationMode" mode="none|border|all"/> approach. But on top of the three explicit values there are also "yes", "no", and "initial" which are meant to do the right thing™ automatically:

  • "yes" is just an alias for "full"
  • the behavior of "no" depends on keepBorder and whether the window uses client side decorations
  • the behavior of "initial" depends on whether the window uses client side decorations

I did not yet change anything about ToggleDecorations. But with this approach there is a straight forward option: <action name="ToggleDecorations" mode1="value" mode2="value"/> toggles between mode1 and mode2 (i.e. new_mode = current_mode == mode1 ? mode2 : mode1). If they are not both provided, the old 3-state behavior is used instead.

While very similar, I prefer this approach over #1725 because it provides both a high-level and a low-level API for users. The defaults make sense, but users still have full control over the exact behavior.

@ahesford
Copy link
Member

The six-way selection here is far too complicated and overloaded. This is a non-starter for me. We aren't trying to be Turing complete with window decorations!

In #1725, adding a keepBorders override to Undecorate and implementing it correctly provides conceptual simplicity: remove a title bar if one is shown and, if keepBorders is not "yes", also remove the borders if they are shown. Adding an optional mode argument to Decorate, which may be something like borders|all, complements that by allowing you to explicitly set a desired state right out of the gate.

At this point, you now have four competing PRs open to adjudicate your decoration issue. Please let discussion percolate on that issue and await a consensus before opening any more alternatives, and maybe use a few of your alternatives for awhile to get a sense of what you think feels right before proposing all of them for inclusion.

@ahesford ahesford marked this pull request as draft April 19, 2024 12:16
@xi
Copy link
Contributor Author

xi commented Apr 20, 2024

Closed in favor of #1733

@xi xi closed this Apr 20, 2024
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.

None yet

2 participants