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

Branch
shellClientFullScreenable
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 12822
Build 12840: arc lint + arc unit
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 59804

View Options

shell_client.cpp

Loading...