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

[v1.0.0b] - Can't have empty instructions/ingredients #2003

Closed
5 tasks done
PaddiM8 opened this issue Jan 8, 2023 · 9 comments · Fixed by #2008
Closed
5 tasks done

[v1.0.0b] - Can't have empty instructions/ingredients #2003

PaddiM8 opened this issue Jan 8, 2023 · 9 comments · Fixed by #2008
Labels
bug: confirmed bug Something isn't working good first issue Good for newcomers

Comments

@PaddiM8
Copy link

PaddiM8 commented Jan 8, 2023

First Check

  • This is not a feature request
  • I added a very descriptive title to this issue.
  • I used the GitHub search to find a similar issue and didn't find it.
  • I searched the Mealie documentation, with the integrated search.
  • I already read the docs and didn't find an answer.

What is the issue you are experiencing?

If I delete the default instructions or the default ingredient, they reappear after I save and refresh. This means that it isn't possible to store dishes without any instructions. I don't have a recipe for all dishes, but I still want to store them in order to use them for meal planning and such.

I guess this would be solved if it was possible to customize the default values, but I haven't found a way to do that.

Deployment

Docker (Linux)

Deployment Details

No response

@hay-kot
Copy link
Collaborator

hay-kot commented Jan 8, 2023

That is super annoying. Best course of action is to probably eliminate the defaults all together and just provide some in-browser help for information on what's supported in the markdown format.

@hay-kot hay-kot added bug Something isn't working bug: confirmed labels Jan 8, 2023
@hay-kot hay-kot added this to the v1.0.0beta-RC1 milestone Jan 8, 2023
@hay-kot hay-kot assigned hay-kot and unassigned hay-kot Jan 8, 2023
@hay-kot hay-kot added the good first issue Good for newcomers label Jan 8, 2023
@PaddiM8
Copy link
Author

PaddiM8 commented Jan 8, 2023

That is super annoying. Best course of action is to probably eliminate the defaults all together and just provide some in-browser help for information on what's supported in the markdown format.

Hm yeah. I have some ideas for this, and I could try to implement it. How about removing the defaults, but maybe create an example recipe automatically when a new group is created? This way you could look at how that recipe was made and it might also help users get a better understanding of the interface right away, before creating any recipes of their own.

It might also be useful to have an info button above the recipe instruction field, that opens a markdown guide dialog? Not sure what would be best here though.

@zierbeek
Copy link
Contributor

zierbeek commented Jan 8, 2023

Oh that would be nice. A tooltip for for example "how to show a video" or "how to link other recipes"

@fleshgolem
Copy link
Contributor

The defaults really are not the problem here, there is just a subtle bug in the __init__ method of RecipeModel, caused by empty lists being falsy, that will lead to this behaviour. I can provide a fix for it in short time

@PaddiM8
Copy link
Author

PaddiM8 commented Jan 8, 2023

@fleshgolem Are the defaults needed though? I find it a bit annoying to have to remove them all the time. They're good for demonstrative purposes, but that could be done with a default recipe.

@hay-kot
Copy link
Collaborator

hay-kot commented Jan 8, 2023

The defaults really are not the problem here, there is just a subtle bug in the init method of RecipeModel, caused by empty lists being falsy, that will lead to this behaviour. I can provide a fix for it in short time

I agree that the core problem is not the defaults, it's a logical error.

Are the defaults needed though? I find it a bit annoying to have to remove them all the time. They're good for demonstrative purposes, but that could be done with a default recipe.

Though this point about UX is a pretty valid one. I think there's better ways to accomplish what I'm trying to do.

Happy to accept a PR to fix the initial issue here, and another on updating the UX issues, or one together.

@fleshgolem
Copy link
Contributor

Yeah sure, that both makes sense. I will provide a fix on the issue itself and you can make a decision on how the UX here should be handled in general

@PaddiM8
Copy link
Author

PaddiM8 commented Jan 8, 2023

That makes sense. I'll make a quick PR for what I had in mind!

@hay-kot
Copy link
Collaborator

hay-kot commented Jan 8, 2023

Thanks @fleshgolem!

See

for UX discussion/tracking

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: confirmed bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants