Differential D18295

CTRL+Q should only close current window, not all windows
AbandonedPublic
Actions

Authored by cullmann on Jan 16 2019, 12:58 PM.

Details

Reviewers
drosca
krishremya
Group Reviewers
Falkon
Summary

CTRL+Q should only close the current window, not all windows.
Bug 401495

Diff Detail

Repository
R875 Falkon
Lint
Lint Skipped
Unit
Unit Tests Skipped
krishremya created this revision.Jan 16 2019, 12:58 PM
Restricted Application added a subscriber: falkon. · View Herald TranscriptJan 16 2019, 12:58 PM
krishremya requested review of this revision.Jan 16 2019, 12:58 PM
SGOrava added a subscriber: SGOrava.Jan 16 2019, 1:52 PM
SGOrava added inline comments.
src/lib/app/browserwindow.cpp
469

Why do you need to change shortcut to close current tab ?

krishremya marked an inline comment as done.EditedJan 17 2019, 2:46 PM

In the bug report, it says to close the current window using CTRL+Q. So I just changed the shortcut for closing the current tab.

drosca requested changes to this revision.Jan 17 2019, 6:12 PM
drosca added a subscriber: drosca.

Actually I myself am not really fan of closing only the window with Ctrl+Q, there is Alt+F4 for that.

But in any case, if it is to be added, it must be optional and disabled by default.

src/lib/app/browserwindow.cpp
469

Yes, there is no reason to change this shortcut.

This revision now requires changes to proceed.Jan 17 2019, 6:12 PM
krishremya updated this revision to Diff 49793.Jan 18 2019, 9:19 AM

CTRL+Q added as an optional shortcut for closing the current tab instead of closing all windows.

krishremya added a comment.Jan 20 2019, 8:25 AM

Can anyone of you please review it.

ach added a subscriber: ach.EditedJan 20 2019, 11:06 AM

-1 from my side.

Ctrl-Q is standard shortcut for exit application. If one wants to close a window, use Alt-F4 or (x) button in upper right. The hole behaviour of a multi-mainwindow app versus several single main window apps is unfortunately not properly definded. But IMHO this should be reassiged to the HIG Group before every multi-mainwindow application implements it's own behaviour.

Just my 2 cents.

The Ctrl-Q use case is: I use 'start falcon with the the windows and tabs as on close' . This implies I need a way to close all windows at once. Otherwise flacon will not (re)start with several windows.

drosca added a comment.Jan 21 2019, 9:21 AM

As I already wrote, it has to be added as new option.

cullmann requested changes to this revision.Jan 26 2019, 3:37 PM
cullmann added a subscriber: cullmann.

Set right flag ;=) this needs a revision, as that should be an option.

This revision now requires changes to proceed.Jan 26 2019, 3:37 PM
SGOrava added a comment.Jan 26 2019, 3:42 PM

After along thinking I believe this is exact example of what should be handled by Keyboard Shortcut Manager and not directly in the code.

So, instead of trying to go around all possible situations when this shortcut should or should not be active, we should work on implementing Keyboard Shortcut Manager.

Since this is the case, are there any volunteers ?

cullmann added a comment.May 22 2019, 5:01 PM

It seems nobody takes this up, shall we close it until a new proper request is submitted?

cullmann commandeered this revision.Jul 8 2019, 9:18 PM
cullmann edited reviewers, added: krishremya; removed: cullmann.

Closing this at the moment until somebody steps up for it.

cullmann abandoned this revision.Jul 8 2019, 9:18 PM

Revision Contents
Changeset List

PathPackages
Msrc/lib/app/browserwindow.cpp (14 lines)

Diff 49628

View Options

src/lib/app/browserwindow.cpp

Loading...