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

Allow custom reminders/times #183

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

Conversation

alexiri
Copy link
Contributor

@alexiri alexiri commented Nov 15, 2024

Let the user configure if they want reminders or not, and how far in advance. Also let them define those as overrides per category.

@ThiefMaster note that this still needs some work, and I'm hoping you can give me a hint. My idea for the overrides is to have a single field where the user could define the status, reminder and/or reminder_time per category. However, I can't figure out how to get MultipleItemsField to have a validator so that at least one of those values is filled in. Also, if the user doesn't specify a value for a particular variable, it should be passed as a None, otherwise _get_status/_get_reminder can't tell the difference between an actual override and a blank. Perhaps this is too complicated for MultipleItemsField as it's currently implemented?

@ThiefMaster
Copy link
Member

Perhaps this is too complicated for MultipleItemsField as it's currently implemented?

Yes, it's a rather simple field. More fancy things typically use a react-based widget nowadays. But that's probably overkill here... And I think leaving a field empty using the default would be fine. There's nothing where overriding with an empty string makes sense. And for reminder minutes, 0 would be an override, and empty would be using the default. That's already possible.

I don't think "must fill at least one" validation is necessary either - the override simply won't do anything if all the fields are empty... Just like an override with an invalid category id won't do anything. (But see my other comment, doing the validation would probably not be hard either.)

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.

2 participants