Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
xai opened this issue Feb 1, 2023 · 4 comments
Open

Comments

@xai
Copy link

xai commented Feb 1, 2023

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
Copy link
Member

hedayat commented Feb 1, 2023

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
Copy link
Author

xai commented Feb 1, 2023

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
Copy link
Member

hedayat commented Feb 4, 2023

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
Copy link
Contributor

mwilck commented Apr 4, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants