projecthamster / hamster-shell-extension

Shell extension for hamster
http://projecthamster.org
GNU General Public License v3.0
217 stars 92 forks source link

Stop tracking when destroy signal is received (e.g., due to screen lock) #354

Open xai opened 1 year ago

xai commented 1 year ago

Summary When the screen is locked in a gnome-session, it would be logical that time tracking is stopped as well by the extension. This was also the behavior of the legacy hamster applet/indicator. Currently, tracking continues.

How to reproduce

  1. Start an activity
  2. Lock screen
  3. Unlock screen again

Observed behavior: hamster is still tracking the activity from before locking the screen Expected behavior: tracking the activity is stopped by locking the screen

hedayat commented 1 year ago

Thank you for your interest and for the PR. I'll wait to see what others have to say. But this is my opinion:

While I can see that such behavior can be desired in some workflows, it is not always the case. Actually, this is usually not the case for myself, because I don't necessarily use Hamster to track my direct activities with the system, but I track my activity which might also include locking the system and thinking about something for awhile.

So, if such behavior is to be implemented, I suggest it to be optional, and to be disabled by default so that the default behavior is not different from before. And to be honest, I guess the actual feature should be implemented in Hamster itself, with the extension just notify Hamster about the system state if it cannot detect it by itself, and let Hamster decide if tracking should be stopped or not.

I'd say its better for the extension to merely provide gnome shell integration, but not its own features in addition to what Hamster itself provides. This allows such things to be consistently available in any DE.

xai commented 1 year ago

Thanks for considering this issue, @hedayat!

So, if such behavior is to be implemented, I suggest it to be optional, and to be disabled by default so that the default behavior is not different from before.

You are right and I agree about making it optional, as clearly workflows and use cases differ.

Our users, coming from the legacy hamster applications, expect the behavior as stated in the ticket, but sure - default behavior changes over time (especially during a major rewrite) and I sure have no objections about making the legacy behavior an optional one.

And to be honest, I guess the actual feature should be implemented in Hamster itself, with the extension just notify Hamster about the system state if it cannot detect it by itself, and let Hamster decide if tracking should be stopped or not.

I'd say its better for the extension to merely provide gnome shell integration, but not its own features in addition to what Hamster itself provides. This allows such things to be consistently available in any DE.

Sounds very reasonable to me.

I cannot promise a PR for hamster as the current extension patch at least partly fulfills the needs of my employer ("partly" because we do still have an issue with logout/shutdown, gnome-shell does not seem to send the destroy signal anymore in these cases and we need a solution for this as well, so there's that ...).

If you don't mind, I would rephrase the issue + switch my PR to a draft and we could just keep this open and try to work on a solution that aligns more with the design of your project and its current default behavior. I cannot promise to contribute respective PRs for hamster, but I am very interested in helping (we do use hamster in our company for time tracking and also intend to roll out this extension as a replacement for the legacy indicator), and maybe others also have similar demands and can help out here.

Edit: I could not mark my PR as draft, so I edited the title as WIP to make it clear that there is a discussion and that it should not be merged as is.

hedayat commented 1 year ago

Thanks a lot for you contribution and understanding. I hope to see this feature going forward, I'm sure others will find it useful too.

mwilck commented 1 year ago

Hamster used to have support for this in the past. It lost the ability to track screen lock events with 3.0-rc1. See https://github.com/projecthamster/hamster/issues/568.