-
Notifications
You must be signed in to change notification settings - Fork 410
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
Fix edit frequency #1852
Fix edit frequency #1852
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. The "every" should show when showing an existing frequency
"hour" by itself doesn't make sense.
Also just noticed this issue, "hours" is still accepted and is a confusing unit "every hours"
freq.mov
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works great, minor comments
@khaliqgant , I have just updated my PR, to check for time units such as "every hours", "every days", "every mins" . It was also accepting "every mins" as an input which is a value less than 5 for minutes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the change requested by Khaliq were unrelated to this PR and it has introduced bugs unfortunately.
- Adding a space before
every
will add an other every - Adding a space between
every
andhours
make the cursor jump to the end - Saving
every hours
complains about the missing unit and revert the input value
Describe your changes
Display frequency whenever the user clicks on Edit