[go: nahoru, domu]

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

Ingredients not showing in editor when hiding them in step #2704

Closed
stleusc opened this issue Oct 30, 2023 · 11 comments
Closed

Ingredients not showing in editor when hiding them in step #2704

stleusc opened this issue Oct 30, 2023 · 11 comments

Comments

@stleusc
Copy link
stleusc commented Oct 30, 2023

Tandoor Version

latest

Setup

Docker / Docker-Compose

Reverse Proxy

No reverse proxy

Other

No response

Bug description

This is related to #2539 and #2266 and #2558

Currently, when hiding the step ingredients and only showing them on top everything looks good in the recipe view.
However, when editing the recipe it makes no sense to hide the ingredients in the editor as well....

Example:

image

When opening in edit mode... last step
image

Only AFTER clicking the + Ingredients button this shows up
image

Obviously in the editor it makes no sense to hide the data!
It is impossible to see which step (or more) has the hidden ingredients.

I would expect the ingredients to be visible in the editor regardless of the setting to hide/show the step ingredients.

To still indicate that they are hidden one could show that crossed out eye icon next to the heading 'Ingredients'.

@srwareham you were the original author of this great addition. Do you think this could be fixed?

Relevant logs

No response

@srwareham
Copy link
Contributor
srwareham commented Nov 1, 2023

Just glimpsing at it, I think changing the logic here would update the behavior.

// set default visibility style for each component of the step
this.recipe.steps.forEach((s) => {
this.$set(s, "time_visible", s.time !== 0)
// ingredients_visible determines whether or not the ingredients UI is shown in the edit view
// show_ingredients_table determine whether the ingredients table is shown in the read view
// these are seperate as one might want to add ingredients but not want the step-level view
this.$set(s, "ingredients_visible", s.show_ingredients_table && (s.ingredients.length > 0 || this.recipe.steps.length === 1))

My PR changed the logic from

this.$set(s, "ingredients_visible", s.ingredients.length > 0 || this.recipe.steps.length === 1) to

this.$set(s, "ingredients_visible", s.show_ingredients_table && (s.ingredients.length > 0 || this.recipe.steps.length === 1))

If you just drop the s.show_ingredients_table && I think it'd introduce the behavior you're looking for. Would you want to try that change and test the various flows to confirm they make sense? The three dots menu option for show/hide step ingredients was a known not-great UX and was intended mostly to be a stopgap until a refactored design. Not sure if that's still planned.

@stleusc
Copy link
Author
stleusc commented Nov 1, 2023 via email

@srwareham
Copy link
Contributor

I'm a bit rusty but I think the gist of it is you need to download the project's source code, modify that one line, compile/build a development version, then open that development version in your web browser to do testing. The project's documentation has steps here.
I think the most relevant steps are:

  1. Open a terminal/command line where you want to save the source code
  2. Download the source code for Tandoor git clone https://github.com/TandoorRecipes/recipes.git
  3. Make sure you have python, yarn, and django installed on your system (steps are operating system dependent)
  4. In a terminal, navigate to the recipes directory you git cloned by typing cd recipes
  5. pip install -r requirements.txt
  6. python manage.py migrate
  7. python manage.py runserver
  8. Open another terminal in the recipes directory and then cd to the vue directory
  9. yarn install
  10. yarn serve
  11. Enter the web address the last command returns in your web browser (something like localhost:XXXX) where the Xs are numbers you also need to type). This should then get you to a working instance of tandoor you can play with.

If you further modify the code while step 7 and 10 are still running they should automatically take effect (with slight delay). If you want to kill them and test a change later on you only need to repeat steps 7 onward (starting in the recipes directory

@stleusc
Copy link
Author
stleusc commented Nov 2, 2023 via email

@srwareham
Copy link
Contributor

I'll confess I'm not well versed in the space but I suspect not. Here's a thread where I was asking about how to build this back when I was trying to learn how to implement this in the first place, if interesting: #2524

@stleusc
Copy link
Author
stleusc commented Nov 2, 2023

Thanks!
I'll look into that. Would've been nice to be able to tweak the running docker image haha.

@smilerz
Copy link
Collaborator
smilerz commented Nov 2, 2023

Thanks! I'll look into that. Would've been nice to be able to tweak the running docker image haha.

In order to reduce the container size none of the development libraries are included, so changes to the front-end can't be modified within the container.

@stleusc
Copy link
Author
stleusc commented Nov 2, 2023

@smilerz
do those steps work:

clone repo
change the vue file as described
run docker build .

Or do I need to do more to compile it before building the docker image? I am 99.999999% sure that the change as described works, so I am ok with locally skipping a whole debug/development setup

@smilerz
Copy link
Collaborator
smilerz commented Nov 2, 2023

@smilerz do those steps work:

clone repo change the vue file as described run docker build .

Or do I need to do more to compile it before building the docker image? I am 99.999999% sure that the change as described works, so I am ok with locally skipping a whole debug/development setup

You will need to

cd vue
yarn install
yarn build

before running docker build - but otherwise, yes I think that should work.

@vabene1111
Copy link
Collaborator

thanks for the efforts to fix this, I kind of remember this to work differently but need to take a look

@stleusc
Copy link
Author
stleusc commented Dec 5, 2023

never mind, changed it locally....

@stleusc stleusc closed this as completed Dec 5, 2023
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

No branches or pull requests

4 participants