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

Option to show a full disk timer #129

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

vin047
Copy link

@vin047 vin047 commented Aug 26, 2022

Instead of just a blue arrow and line around the clock, this provides the option to fill the entire clock face. It makes the visual countdown more prominent which is useful for people who have ADHD and other such attention issues – visual countdowns help externalise the passage of time which helps us pay more attention to it.

Enabling this option also hides the digital timer since it didn't look aesthetically pleasing IMO.

Depends on #125.

@height
Copy link

height bot commented Aug 26, 2022

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

@karbassi
Copy link
Collaborator

Thanks for all the PRs. I'm excited to see another dev adding to the project! 🎉

Could you make each PR self-containing without the requirement of another PR. That way we can review the PR on the changes (what it fixes or what new features it implements) without having to merge another PR. I understand there will be repeat code.

@vin047
Copy link
Author

vin047 commented Aug 27, 2022

Hey @karbassi, thanks for the warm welcome. Glad I can contribute to this project!

Screenshots

Screenshots below for showing a full disk timer. Note that it auto hides the digital timer when enabled as it, IMO, didn't look so good with it being shown at the same time. Auto-hiding the timer isn't a preference change though and reverts back to being shown when this option is disabled.

Current look

current

This PR

change

Its an optional setting, and defaults to the current behaviour, so shouldn't affect other people if they don't like it.

PR dependency

Could you make each PR self-containing without the requirement of another PR. That way we can review the PR on the changes (what it fixes or what new features it implements) without having to merge another PR. I understand there will be repeat code.

The first 2 commits of all the PRs I've submitted are actually all the same (adds the View menu option and saves/loads preferences from UserDefaults) – in other words, the PRs are already self-contained and can be merged without needing #125. I only split it up like that to make it easier for code review. But happy to close that PR if you prefer.

Let me know if this is ok. I'll then fix up the lint issues that have been flagged.

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 this pull request may close these issues.

None yet

2 participants