D16897: Allow windows to be closed from the window switchers

[Nathaniel Graham]

ngraham abandoned this revision.
ngraham added a comment.


  Continued at 
https://invent.kde.org/plasma/kdeplasma-addons/-/merge_requests/5 and 
https://invent.kde.org/plasma/plasma-workspace/-/merge_requests/115

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: graesslin, davidedmundson, abetts, Zren, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, ragreen, ZrenBot, 
ngraham, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, 
ahiemstra, mart

D16897: Allow windows to be closed from the window switchers

[Martin Flöser]

graesslin added a comment.


  In D16897#359912 , @ngraham wrote:
  
  > In D16897#359911 , @graesslin 
wrote:
  >
  > > In D16897#359900 , @ngraham 
wrote:
  > >
  > > > Good point, guys. I'll start with `StandardKey.Close` for now and see 
about making the KWin changes later (I'll admit I'm a bit intimidated by KWin's 
codebase!).
  > >
  > >
  > > I would prefer if the standardKey.Close is not used. This is an 
in-application shortcut used by Qt, but not necessarily by GTK or other 
toolkits. It is not used by the window manager. Please don't mix in-application 
shortcuts and global shortcuts. It would be confusing to users.
  >
  >
  > Isn't the in-application shortcut for window closing exactly what the user 
would expect? From another perspective, the implementation of this feature in 
other window managers and other platforms (macOS, GNOME) uses the same 
window-closing shortcut in the switcher, so it seems that this is what is 
expected and common.
  
  
  And how do you know the in-application shortcut of a window? What's for 
example the in application close window shortcut of Konsole or xterm? There 
simply is no universal shortcut. Also our default shortcut for Qt application 
is ctrl+q which simply is impossible to press in an alt+tab session.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: graesslin, davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Nathaniel Graham]

ngraham added a comment.


  In D16897#359911 , @graesslin 
wrote:
  
  > In D16897#359900 , @ngraham 
wrote:
  >
  > > Good point, guys. I'll start with `StandardKey.Close` for now and see 
about making the KWin changes later (I'll admit I'm a bit intimidated by KWin's 
codebase!).
  >
  >
  > I would prefer if the standardKey.Close is not used. This is an 
in-application shortcut used by Qt, but not necessarily by GTK or other 
toolkits. It is not used by the window manager. Please don't mix in-application 
shortcuts and global shortcuts. It would be confusing to users.
  
  
  Isn't the in-application shortcut for window closing exactly what the user 
would expect? From another perspective, the implementation of this feature in 
other window managers and other platforms (macOS, GNOME) uses the same 
window-closing shortcut in the switcher, so it seems that this is what is 
expected and common.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: graesslin, davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Martin Flöser]

graesslin added a comment.


  In D16897#359900 , @ngraham wrote:
  
  > Good point, guys. I'll start with `StandardKey.Close` for now and see about 
making the KWin changes later (I'll admit I'm a bit intimidated by KWin's 
codebase!).
  
  
  I would prefer if the standardKey.Close is not used. This is an 
in-application shortcut used by Qt, but not necessarily by GTK or other 
toolkits. It is not used by the window manager. Please don't mix in-application 
shortcuts and global shortcuts. It would be confusing to users.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: graesslin, davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Nathaniel Graham]

ngraham planned changes to this revision.
ngraham added a comment.


  Good point, guys. I'll start with `StandardKey.Close` for now and see about 
making the KWin changes later (I'll admit I'm a bit intimidated by KWin's 
codebase!).

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: graesslin, davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Martin Flöser]

graesslin added a comment.


  I agree with David: please don't hardcode the key. Please don't use "W" - it 
has no meaning in a window manager, it's just the shortcut in some 
applications, others use Ctrl+Q and localized it could be something completely 
different.
  
  I suggest to implement this in KWin and intercept the shortcut hold by kwin - 
if it matches close the window.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: graesslin, davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[David Edmundson]

davidedmundson added a comment.


  You can do control+w with
  
  if (event.matches(StandardKey.Close)) {
  }
  
  alt+f4 would need some exposing on the kwin side as that's not a "standard 
key" exposed via the QPT

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[David Edmundson]

davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  you can't hardcode the key combos

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg, davidedmundson
Cc: davidedmundson, abetts, Zren, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Andres Betts]

abetts added a comment.


  +1

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg
Cc: abetts, Zren, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Nathaniel Graham]

ngraham added a subscriber: Zren.
ngraham added a comment.


  I'm also having trouble getting the same approach to work for the default 
Breeze switcher, which lives in plasma-workspace. Keyboard navigation with the 
arrow keys works, so the event handler is functional. But I can't seem to 
target the model for some reason.

REPOSITORY
  R114 Plasma Addons

REVISION DETAIL
  https://phabricator.kde.org/D16897

To: ngraham, #plasma, #vdg
Cc: Zren, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16897: Allow windows to be closed from the window switchers

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: Plasma, VDG.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Implements the ability to close windows from the various non-default window 
switchers using the [F4] or [w] keys.
  
  CCBUG: 308331

TEST PLAN
  Apply patch, Alt+tab to activate window switcher, hit [F4] or {w}. The 
current window closes.
  
  This works for all but the "Informative" switcher, which is still broken for 
all keyboard navigation (unrelated to this patch; once that's fixed, this will 
start working).

REPOSITORY
  R114 Plasma Addons

BRANCH
  close-windows-from-windowswitcher (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D16897

AFFECTED FILES
  windowswitchers/big_icons/contents/ui/main.qml
  windowswitchers/compact/contents/ui/main.qml
  windowswitchers/informative/contents/ui/main.qml
  windowswitchers/present_windows/contents/ui/main.qml
  windowswitchers/small_icons/contents/ui/main.qml
  windowswitchers/text/contents/ui/main.qml
  windowswitchers/thumbnails/contents/ui/main.qml

To: ngraham, #plasma, #vdg
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart