Differential D18132

Rework ShellClient fullscreen control
ClosedPublic
Actions

Authored by romangg on Jan 9 2019, 8:08 PM.

Details

Reviewers
None
Group Reviewers
KWin
Maniphest Tasks
T11098: Replace Screens class
Commits
R108:1347610bb935: Rework ShellClient fullscreen control
Summary

The ShellClient::setFullScreen method was a direct copy of the Client
version.

Straighten out the function logic now and align with modern coding style.

In short we check:

  1. what the manual override window rule wants,
  2. if there is a change at all with this,
  3. if such a change is possible.

And do:

  1. (un-)set the fullscreen,
  2. emit the changed signal.
Test Plan

Manually, autotests pass.

Diff Detail

Repository
R108 KWin
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
romangg created this revision.Jan 9 2019, 8:08 PM
Restricted Application added a project: KWin. · View Herald TranscriptJan 9 2019, 8:08 PM
Restricted Application added a subscriber: kwin. · View Herald Transcript
romangg requested review of this revision.Jan 9 2019, 8:08 PM
zzag added a subscriber: zzag.Jan 9 2019, 8:28 PM

Please capitalize "screen" in "fullscreen".

shell_client.h
88–90 ↗(On Diff #49102)

Unrelated change.

romangg added a comment.Jan 10 2019, 11:50 AM
In D18132#390113, @zzag wrote:

Please capitalize "screen" in "fullscreen".

I'm thinking about going the other way. Changing every occurrence of FullScreen to Fullscreen. Wikipedia, the Wayland docs and xdg-shell stable all write the word together. So camel case S is not in order.

zzag added a comment.Jan 10 2019, 12:33 PM
In D18132#390453, @romangg wrote:

I'm thinking about going the other way. Changing every occurrence of FullScreen to Fullscreen. Wikipedia, the Wayland docs and xdg-shell stable all write the word together. So camel case S is not in order.

Argh, consistency will be messed up. Currently, capitalized "screen" is more popular in KWin, though in KWayland "screen" has lowercase.

zzag added a comment.EditedFeb 8 2019, 2:34 PM

Given that we'd like to update the fullscreen state same way as the maximize state, I think we can live with the current control flow in ShellClient:;setFullScreen (for now).

romangg updated this revision to Diff 59804.Jun 14 2019, 4:56 PM
romangg marked an inline comment as done.
  • Rebase on master
  • Remove function definition in header
romangg added a comment.Jul 8 2019, 10:43 AM

Merging now to not stall for too long the series without good reason.

This revision was not accepted when it landed; it landed in state Needs Review.Jul 8 2019, 10:44 AM
Closed by commit R108:1347610bb935: Rework ShellClient fullscreen control (authored by romangg). · Explain Why
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mshell_client.cpp (31 lines)

Diff 61326

View Options

shell_client.cpp

Loading...