feat: Automatic instruction timers

[codetakki]

What type of PR is this?

(REQUIRED)

• feature

What this PR does / why we need it:

(REQUIRED)
This pr is my suggestions for timers that are added directly to steps, making the timer feature more automatic.
I suggest we attempt to parse the instructions text to find out timers. The regex is currently very basic, and uses locales to support single and plural of minute in different languages. I hope this catches most cases. Timers with hours could easily be added to. For seconds i dont think its necessary.
I have placed the timer below the step, as i don't like the suggestion of adding it inline in the text, as it can be hard to click, does not look good and easy to miss.
The timer is simple, but does allow for reducing or adding some seconds if needed.
It might also be a option to only show it during cooking mode, but as cooking mode only is available if i link products to steps i don't like this restriction.

images

15 min timer detected

Timer is running

Timer is paused

Timer is done, Clock shakes to call attention

Multiple timers are supported:

Which issue(s) this PR fixes:

(REQUIRED)

Special notes for your reviewer:

(fill-in or delete this section)
This is just a suggestion, but having a timer in directly in the instructions would be a great improvement. I'm happy to adjust after feedback.

Testing

Checkout pr.
Go to a recipe
include "5 min" in a instruction step
You should see a timer appear under the instruction.
It should be able to be started, paused, and reset.
(fill-in or delete this section)

[Jaykurb]

This looks great!! Nice work!

[boc-the-git]

Haven't looked at the code yet, but like the look of this!

Your description mentions clock shaking. Do they make noise? I think the current timer does.

[Kuchenpirat]

You only have to provide the translation in the enUs file.
The rest is handled by crowdin

[boc-the-git]

Please remove all translation files other than en-US.
Translation are done via Crowdin.

[codetakki]

Haven't looked at the code yet, but like the look of this!

Your description mentions clock shaking. Do they make noise? I think the current timer does.

Yes it makes the same sound as the existing one

[codetakki]

I removed the locales changes except for en. I'm fluent in Swedish, so could that back in, or should that to be done in Crowdin later?

[Kuchenpirat]

Thanks 👍
Everything besides the base en_US file should be done via crowdin. So don't add any of them back in. You are more than welcome to provide translations via crowidn as soon as this PR is merged.

[codetakki]

As I want locales to work during development, how would you suggest I try them out, should i just ignore my translation files?

[Kuchenpirat]

i am not sure if that works for files that are already tracked via github.
If not you can just add the translations for swedish and other languages you need and we will ask you to remove them once this is ready to merge.

[boc-the-git]

You can have the Swedish language file in your Dev environment and just not commit it. Nothing complicated to do there.

[codetakki]

Fair enough 😄

[codetakki]

Thats super! Do you think we can add an option to trigger the parsing for all recipes manually? That way we can display a warning that it will take a while and even a progress indicator, and we can get the timers on existing recipes. We could also add a button under specific recipes to parse its timers?

Does the parser use all the locales for parsing? We might be able to speed it up by allowing the user to limit what languages are used?

[michael-genson]

Do you think we can add an option to trigger the parsing for all recipes manually

Yeah this isn't a bad idea. We could add some sort of startup task which prompts the user about the new feature. Alternatively we could add a button in the recipe to trigger it on a per-recipe basis.

Does the parser use all the locales for parsing?

Great point, hadn't thought of that. It definitely does

[michael-genson]

Oh you literally suggested the button on each recipe in the next sentence. Missed that, haha

[codetakki]

I have added ui to change timers:

Is there a existing endpoint for parsing the timers for all/singe recipes?
Do we want to parse the timers when a step is edited?

[michael-genson]

I'm open for feedback on that one. I think either we need to parse everything on migration (which means we need to figure out how to make it faster, or telegraph to the user after the app loads up to trigger it manually one-time) or have an endpoint to parse the entire recipe.

I'm leaning towards "figure out how to make it go faster" I just haven't looked into it too much. You can try playing around if you have the time: https://github.com/scrapinghub/dateparser

[codetakki]

i ran some simple tests and tried to run the dateparse 100 times the same string that around the length of a instruction.
It seems that limiting the number of locales improves the times ALOT.
Parsing "this step will take around 5 mins, and then it will be done. Add 10 hours to your timer if not done and then add the eggs" * 100 Without defining locales took around 3.9 s
With languages set to ["en", "sv"] it took 0.15s to parse it 100 times
So i would say if we start there, it might be fast enough to work at migration. Is int there a global set language that we can use? That plus English should cover most cases

[michael-genson]

Awesome, I should have time today or tomorrow to give that a whirl.

Off the top of my head there's no language setting the backend can reach (without a request from the frontend) but I'm thinking we can do something like pre-scan recipes for language and then use those

[codetakki]

Hmm the date parser did already have function to do that, so it might be that it's guessing the language each time. So the improvement there would be that we do it once per recipe.

[michael-genson]

I went and added this to the migration. We take 20 random instructions (each from a unique recipe) and try to detect languages from them. We then parse all ingredients using those detected languages.

In my testing it was pretty fast (10-15 seconds).

[codetakki]

Not bad! When a new recipe is added it uses all of them like before?

[michael-genson]

Yup, I only added it to the migration, otherwise it's the same as before.

[codetakki]

Alright, so now we have a ui, functional backend and UI for editing timers.

What would be left to change/add? I feel like additional features could be added later if wanted, as this seams pretty good for a first version.

[codetakki]

i was just reminded of the mass import feature, there we might still see long loading times. i have personally imported over 100 recipes once. And it already felt like that might have frozen the system. But then again, i dont know how common that is.

[michael-genson]

For the sake of keeping this PR in a reasonable scope I think we can leave it for now. That's something we can easily address later.
In terms of getting this merged I think it's good to go, pending a review

[michael-genson]

It looks like you'll have to fix the merge conflict in RecipePageInstructions.vue. It's related to the new cook mode changes. I'd also verify the timers look okay in cook mode.

For the poetry conflict, just run poetry lock --no-update and commit whatever that spits out

[codetakki]

Is there any documentation that we want to update?

[hay-kot]

Sorry I'm late to the party on this one but I have some general concerns about functionality here and the overall UX. I think my main questions are

• If I start a timer and close my phone or it goes to sleep what happens to that timer? Does it keep ticking, will it notify me when it goes off?
• If I refresh the page, what happens to the timer?

I've tried implementing this kind of thing before and there was no way to keep the timer running when the phone went to sleep or the browser app was left.

Overall, I think that this restriction makes this kind of change confusing for users and unpredictable. What I would like to avoid is having users filing issues about timers "not working" when implementing a fully functional timer isn't possible.

I know a ton of work has gone into this and I don't want to be the one to say we should throw this all away this late in the PR process, but I'd at least like to have a conversation about how this would get used and what the unexpected limitations are.

[michael-genson]

If I start a timer and close my phone or it goes to sleep what happens to that timer?

Yeah after thinking about it more this is definitely a big concern. As a user I would assume the timer would continue to work, but as a developer I'm pretty sure there's no way to get that to work without an actual app. The issue with refreshing the page is less important to me, but I lock my phone instinctively, and not all devices support wakelock

[codetakki]

I have had the same concern and even looked into a solution but turning up empty-handed, even though pwa have come a long way, having it track time in the background seams impossible/impractical. Unless we implement web notifications.
I want to mention that this issue was also present in the current timer implementation, which presents the same issues.
Mealie does have the feature where it keeps the screen on while looking at a recipe, but more than that will be hard.

I have some things in mind to improve it somewhat:
Looking into: https://www.npmjs.com/package/worker-timers, it might be able to improve it somewhat, but needs support for web workers.
Warn the user first time they use the timer that its not perfect, and should keep the screen on.
Add a "timeStarted" variable that we can double-check time progression, so if we close the app and open it again, as long as that variable is kept (local storage, session etc) we can at least notify the user that the timer has progressed.

Or: Offload the timer to a separate app, using app scheme URL or something.

[codetakki]

I was looking for some kind of feedback to be able to get this pr merged. So let me know what should be done.