New option to hide cursor for DEs other than GNOME and KDE

flameshot-org / flameshot

Powerful yet simple to use screenshot software :desktop_computer: :camera_flash:

https://flameshot.org

GNU General Public License v3.0

23.92k stars 1.52k forks source link

New option to hide cursor for DEs other than GNOME and KDE #3607

Open berkentekin opened 1 month ago

berkentekin commented 1 month ago

This PR adds an option to hide cursor for screenshots taken in DEs other than GNOME and KDE. The checkbox is therefore disabled in GNOME, KDE and non-UNIX systems.
In screenGrabber.cpp, the application decides to use either a DBus interface or the wlroots-based grim program depending on the desktop environment. Unfortunately, from what I understand, KDE and GNOME do not support wlr protocols, and the DBus interface does not provide an option to show or hide the cursor. Therefore, my changes (should) only apply to DEs that can utilize grim.
grim includes the cursor in screenshots with -c argument. This PR basically just allows passing this option.
The cursor is hidden by default.
For now, no CLI options to set this behaviour. If requested, I can mark this PR as WIP and work on that.

Partially resolves flameshot-org/flameshot#3582, flameshot-org/flameshot#604

mmahmoudian commented 1 month ago

Thanks for the PR. I believe the correct course UX is to have only and only one checkbox that works universally on all DEs and OSs. What I mean is that the user should not care which feature they should toggle based on what software stack they are using. So if we cannot turn off cursor in Gnome Wayland or KDE or Windows, we should somehow show a message to the user and inform them why the checkbox is disabled.

In this implementation the Windows, Mac, and BSD users will just get a disabled checkbox without any explanation, and the Gnome and KDE users will get different tooltip.

I believe it is better if

we can show a red text above this checkbox to notify the user
include Mac and BSD in the mix by reversing the if statement condition to if linux -> if wayland -> if not gnome or not kde -> ...

This PR also addresses #604 in a way (giving control over showing cursor in Wayland)

mmahmoudian commented 1 month ago

Aesthetically I like it, but the point is that users rarely check the tooltip for more info. How about adding a ℹ️ or something similar at the end of such items that kind of invite/attract the user to click/hover to get the reason on why this is red.

Btw, thanks for your efforts and responsiveness 👍

berkentekin commented 1 month ago

@mmahmoudian Thank you for your kind words.

On second thought I think adding a text is a better idea precisely because of the reason you've mentioned. I just deleted my comment because my design was such a minor difference that I didn't think it'd be worth mentioning... Besides, whether an unsupported environment actually grabs the cursor or not is outside our control, so informing the user about the issue on first sight resolves the ambiguity of a disabled-but-checked box.

In the end this is what it looks like:

Screenshot_20240514_190908