In some applications like Chromium or Dolphin, it is possible to use middle click to close a tab. This patch brings the similar user experience to Okular.
Details
Diff Detail
- Repository
- R223 Okular
- Lint
Lint Skipped - Unit
Unit Tests Skipped
Please make it configurable, and off by default (otherwise it is too easy to mis-click and close a document).
Do people frequently misclick with the middle mouse button? Most applications with non-permanent tabs allow tabs to be closed with MMB without an option to disable the feature. If it's not a problem for Firefox or Chromium, I don't see it being a problem for Okular.
Do people frequently misclick with the middle mouse button?
It can happen, and more often with a touchpad.
Most applications with non-permanent tabs allow tabs to be closed with MMB without an option to disable the feature. If it's not a problem for Firefox or Chromium, I don't see it being a problem for Okular.
I stopped counting the tabs I accidentally closed in browsers and yakuake (hello D14860: Make middle click closes tab optional...).
I hadn't considered that. By default, you have to tap with 3 fingers to middle click. What are you doing with the touchpad when this happens? Does this happen when your hands are resting or when you try to scroll?
The solution to accidentally clicks is not disabling it by default and making it configurable, but providing the functionality to "restore" a tab after closing it accidentally. Dolphin and Chrome allow me to restore the last closed tab with Ctrl+Shift+T. Does Okular the same?
One use case is clickpads. However, it can happen also with classic mice, especially the small ones where the buttons are close.
And this "solution" does not always work, for example when a page was the result of a form submission, redirection, or other kind of interactivity.
I'm aware, but I wanted to know how the misclick happens. It might point to another problem that needs to be fixed.
I assume you're talking about Libinput's virtual middle mouse button for buttonless touchpads when using the default "areas" click mode. IMO this is a library problem that we should not work around in apps. You can tell Libinput to use "fingers" rather than "areas" mode and then middle-clicking requires three fingers which is much more difficult to do accidentally. Our touchpad KCM allows configuring this, though we don't make it the default because we respect upstream defaults. We could consider overriding that and using the "fingers" click method by default the way we currently override upstream defaults to force better font hinting in the Fonts KCM.
Exactly. If tab closing is undoable, then closing accidentally is no big deal at all.
To prevent some accidental middle-mouse-clicks on the tab bar, we could lock the cursor while middle-mouse-button-zoom.
D21759 would do that. :)
It is possible. However, before getting down to implement it, I am not sure if anyone else agree with this solution.
By the way, what do you think should be the best way to trigger an undo, such as a keyboard shortcut or a menu entry? Do we need some visual elements to tell the user that the tab closing is undoable?
Undoable tab closing seems totally uncontroversial to me. Web browsers have it, Dolphin has it... It's really just a matter of consistency. On that subject, here's how Dolphin does it:
So it would have a menu item + the standard keyboard shortcut. Seems like a pretty decent UI to me.
Remember any UI change as this should have it's UI autotest to make sure things don't break in the future.
Also please remember to use invent.kde.org and not phabricator for okular merge requests
As a sidenote i don't think this makes sense at all, but i don't think allowing tabs in okular makes sense either and i let them in so i'll just say it and then go back to my cave
May I know what the difference is between posting a diff patch here and making a MR at invent.kde.org? I was brought here from Get Involved/development where invent.kde.org was never mentioned.
The KDE Community is transitioning away from Phabricator and towards GitLab. The transition is not yet complete, which is why the documentation still points you towards Phab. Individual apps--such as Okular--have already made the jump as "early adopters", so to speak.
Probably it's fine to keep this here since there's already been some discussion that it would be good not to lose, but please use https://invent.kde.org/kde/okular/merge_requests for future Okular contributions. Thanks!
@ngraham In Okular, when a tab with unsaved changes is about to close, the user will be prompted first. Does this help?
Yes this helps the case that @pino brings up, but it handles a different use case. Most of the time a tab won't have unsaved changes because Okular is primarily a reading app. An "Undo last closed tab" feature is for these other cases.
Based on this feature, can we assume a closed tab has no unsaved changes? If that is the case, we might no need any associated data (except url) to serialize for reopening this tab.
Another thing is, how many closed tabs should be saved in memory?
I think we can assume that, but check in the code. I don't think we necessarily need to save anything in memory except the URL for the document shown in the closed tab.
Thanks, this looks great. It works just fine and the UI seems sane to me. I have some code comments:
shell/shell.cpp | ||
---|---|---|
660 | prefer append() | |
778 | coding style: always use braces | |
778 | m_closedTabUrls.isEmpty() | |
783 | ditto | |
shell/shell.h | ||
175 | A regular old QList of QUrls would seem to do just fine as it doesn't seem that you're using any functionality present in QLinkedList not in QList. | |
shell/shell.rc | ||
8 | Need to bump the version number at the top of this file anything you change anything |
An MR is drafted at invent, see https://invent.kde.org/kde/okular/merge_requests/69 .