SafeEyes icon indicating copy to clipboard operation
SafeEyes copied to clipboard

Plugin for limiting the number of consecutive skips or postpones

Open undefiened opened this issue 2 years ago • 3 comments

I added plugin which allows to limit the number of consecutive times one can postpone or skip break. This is a middle ground between the strict mode which completely disables the skipping/postponing feature, and being able to click skip/postpone indefinitely. There is only one setting which tells how many times in a row one can skip breaks.

When user has skipped X breaks consecutively (without taking any breaks) both skip and postpone buttons become disabled. Taking break completely (without skipping or postponing) resets the counter.

This should somewhat address https://github.com/slgobinath/SafeEyes/issues/320. At the very least it addresses @tuhlaajapoika suggestion https://github.com/slgobinath/SafeEyes/issues/320#issuecomment-826927305 (I also happened to have ADHD, so what @tuhlaajapoika described was exactly my problem: I was automatically clicking skip without even thinking).

There was no existing mechanism which would have allowed me to remove both buttons without making any changes in UI or core, so I added two flags to "context" and added some logic to UI. The intention behind these two flags is to simply disable both buttons for the next break, after which both flags become reset back to False. Not sure if it is the best solution, so if you have some better mechanisms in mind feel free to let me know and I will do it.

Also, I am not sure about wording of settings and plugin hints, so I would appreciate any suggestions.

undefiened avatar May 28 '23 12:05 undefiened

Dear @hartwork and @slgobinath, I looked at the previous PR's and noticed that you are usually the people who resolve them. I just wanted to mention your usernames to make sure that you are aware of this pull request. There is absolutely no time pressure from my side, I'm just making sure that you are aware of this PR in case you didn't receive any notifications.

undefiened avatar Jun 27 '23 11:06 undefiened

Hi @undefiened , Thank you for your contribution. Sorry for the late response I got busy with my life and have little to no time for my FOSS projects. I didn't get a chance to test it but the code looks good to me. Can you please add an icon suitable for this plugin as described here https://github.com/safeeyes/safeeyes-plugins#best-practices ?

FYI: The new URL to download font awesome icons is https://fa2png.app/

slgobinath avatar Jun 28 '23 11:06 slgobinath

Added an icon. I didn't find anything suitable, and 24x24 is a rather limiting size, so I did my best. If there are any better ideas, I will be happy to incorporate them.

No worries at all about not having time, it was not my intention to hurry you, I just was not sure how GitHub notification system works. Please take as much time as needed to reply and review. Thank you very much for all your hard work!

And thank you for giving a link to the safeeyes plugins repo, I somehow missed it in the description. Should I move my plugin to the safeeyes-plugins repo? If yes, then I probably should make two separate PRs, one here to modify the core stuff and one in the third-party plugins repo. Or is it fine to have the plugin here?

Also, I removed the copyright with your username from my plugin.py (I have copypasted the header from your other plugins, so it included your copyright). But I can return it back if needed, I don't really understand the logic of licenses and copyrights, I am happily giving up all my copyrights (if I ever had them) or whatever.

undefiened avatar Jul 08 '23 23:07 undefiened

Hi @undefiened, I am a new maintainer, and I will be happy to test and merge this pull request ASAP. Can you please resolve the conflict in the commit?

I am in favor of merging this into main SafeEyes because it would improve the core functionality of SafeEyes. I feel that the plugins in https://github.com/safeeyes/safeeyes-plugins are not related to the core functionality (Zoom meeting detection, weather forecast, etc).

Also, I removed the copyright with your username from my plugin.py (I have copypasted the header from your other plugins, so it included your copyright). But I can return it back if needed, I don't really understand the logic of licenses and copyrights, I am happily giving up all my copyrights (if I ever had them) or whatever.

Not that I understand how copyrights work either, but I would suggest that you add your own name as the author of this file, and also keep slgobinath's name in case you reused any code written by him.

archisman-panigrahi avatar Jul 10 '24 02:07 archisman-panigrahi

@archisman-panigrahi Thank you very much for taking over the repo! It is very good to hear that this incredible project will continue to be developed.

I did what you have requested, please let me know is something is not up to standards!

undefiened avatar Jul 10 '24 17:07 undefiened

Unless someone finds any regression, I will merge this on Sunday (and release a new version of Safe Eyes).

archisman-panigrahi avatar Jul 11 '24 06:07 archisman-panigrahi

@undefiened I agree that this pull request will fix #320. Thank you very much for your contribution as well as for your patience.

archisman-panigrahi avatar Jul 11 '24 06:07 archisman-panigrahi

This is super helpful. Thanks again! The latest version includes this PR.

archisman-panigrahi avatar Jul 14 '24 14:07 archisman-panigrahi

@undefiened I am in favor of enabling this plugin in the default installation (with 2 allowed skips). Do you know how to do that?

archisman-panigrahi avatar Jul 14 '24 16:07 archisman-panigrahi

@undefiened I am in favor of enabling this plugin in the default installation (with 2 allowed skips). Do you know how to do that?

I made a PR that seems to do that - https://github.com/slgobinath/SafeEyes/pull/603

Although, as I wrote there, I am not sure whether it is a good idea because this plugin introduces a real blocking behavior by default.

undefiened avatar Jul 14 '24 18:07 undefiened

This is super helpful. Thanks again! The latest version includes this PR.

Thank you very much! Now I can start using upstream SafeEyes again :)

undefiened avatar Jul 14 '24 18:07 undefiened