dboehmer / coocook

👨‍🍳🦉 Web application for collecting recipes and making food plans
https://coocook.org/
Other
11 stars 2 forks source link

Bootstrap update to v5 of recipe, dish and purchase list editor #183

Open kuro610 opened 2 years ago

kuro610 commented 2 years ago

Closes #130 Closes #131 Closes #133

kuro610 commented 2 years ago

@dboehmer The update of the purchase list editor makes 1 test failing "change item total" from t/controller_Item.t. As far as I understand the test it should work, but it could be that the test references a wrong form in line 52. So could you please check this.-

dboehmer commented 2 years ago

The project page shows an empty alert box for this test project when not logged in:

image

dboehmer commented 2 years ago

@dboehmer The update of the purchase list editor makes 1 test failing "change item total" from t/controller_Item.t. As far as I understand the test it should work, but it could be that the test references a wrong form in line 52. So could you please check this.-

I tried to change arguments to submit_form_ok to the form with ID total-1 but it still doesn’t work. Maybe WWW::Mechanize doesn’t like the empty <form>s with form elements outside an attribute form="total-1"? I am not used to that syntax, too.

kuro610 commented 2 years ago

Before the update the form only had a name ("total") (which would also not be unique when we have multiple items on our purchase list) and no id. I added the id so that I can use the form elements outside with the form attribute. The unit conversion form works also with this approach (form elements outside the form), but maybe this form is not covered by any test.

kuro610 commented 2 years ago

If I test the total form by hand it works.

kuro610 commented 2 years ago

Here are some information about the form attribute on form elements.

kuro610 commented 2 years ago

The project page shows an empty alert box for this test project when not logged in:

image

I removed the empty alert box.

kuro610 commented 2 years ago

I fixed the test. It looks like that Mechanize didn't support the form attribute on inputs.

I think now is the branch ready to be merged