Formally make app compatible with NC28

[christianlupus]

Topic and Scope

Closes #1928

This marks the app as compatible with NC 28.

Concerns/issues

No tests have been carried out yet.

Formal requirements

There are some formal requirements that should be satisfied. Please mark those by checking the corresponding box.

• I did check that the app can still be opened and does not throw any browser logs
• I created tests for newly added PHP code (check this if no PHP changes were made)
• I updated the OpenAPI specs and added an entry to the API changelog (check if API was not modified)
• I notified the matrix channel if I introduced an API change

[github-actions]

Test Results

552 tests   552 ✔️  19s ⏱️
143 suites      0 💤
    3 files        0 ❌

Results for commit 726c959.

♻️ This comment has been updated with latest results.

[christianlupus]

Hello @seyfeb, could you please have a look at this and check if there are any significant issues I missed? This one should allow us to run with the current master branch of the server.

[seyfeb]

I did some manual testing of the app and noticed following issues. I did not yet have time to cross-check them against an older version to see if they were introduced here or still present in before:

(Tested on Firefox)

General

• Image is distorted in RecipeView (tested with import from https://www.hellofresh.de/recipes/pizzetta-caprese-mit-mozzarella-64feceb9333d6f05dac3634a)

Settings

• Changing Recipe directory fails
• When deleting the number of update interval and entering a new number an error occurs, because it tries to set the interval to the value of the (temporarily) empty field (Fix/1962 error when changing rescan period #1963)

Navigation

• Renaming category fails

AppControls

• Recipe "Search" is not working, "Filter" is working (Recipe search is not working #1964, solved in Fix/1964 recipe search is not working #1965)

RecipeEdit

• The Abort Edit button has an eye as icon (Fix/1966 change abort icon during editing #1971 )
• The value for cooking, prep, and total time are not filled in the respective text input fields (Fix/1970 cooking prep and total time are not filled during edits #1973 )

EditMultiselectInputGroup

• Weird cursor in Select component after selecting a value (Weird cursor in Select component after selecting option #1969)
• Unclear what ---- option is supposed to mean => Accidentially selecting it deletes line (Nutrition removal unclear #1967)
  • If it is supposed to delete a line, my suggestion would be to better remove this option (which is not an actual option) and add a trash can symbol next to the line instead.
• I find it confusing that the order of rows changes when an the option of an already filled row in the middle is changed. This looks like the row is deleted while it is only moved to the end of the list for no apparant reason (Fix jumping order of nutritions #1968)

[seyfeb]

What was the motivation behind rewriting the component?

[christianlupus]

The fact that NcMultiselect was dropped. You are supposed to use NcSelect instead. I tried to make things more robust by avoiding the watcher.

[christianlupus]

Image is distorted in RecipeView (tested with import from https://www.hellofresh.de/recipes/pizzetta-caprese-mit-mozzarella-64feceb9333d6f05dac3634a)

I cannot reproduce. What do you mean by distorted?

With NC27:

With NC28:

This looks rather similar to me.

[codecov]

Codecov Report

Merging #1950 (acc4448) into master (8c04078) will not change coverage.
Report is 6 commits behind head on master.
The diff coverage is n/a.

❗ Current head acc4448 differs from pull request most recent head 726c959. Consider uploading reports for the commit 726c959 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             master    #1950    +/-   ##
==========================================
  Coverage     80.07%   80.07%            
- Complexity        0      765   +765     
==========================================
  Files            92       92            
  Lines          2650     2650            
==========================================
  Hits           2122     2122            
  Misses          528      528            

Flag	Coverage Δ
integration	21.43% <ø> (ø)
migration	5.69% <ø> (ø)
unittests	57.09% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[christianlupus]

Changing Recipe directory fails

I can confirm. This is a bug with the filepicker API. This might affect the guest view as well.

[christianlupus]

Renaming category fails

For some reason, nothing is happening. Neither an error is logged nor a request to the backend is triggered. Further investigation is needed.

[christianlupus]

Recipe "Search" is not working, "Filter" is working

I can confirm this. The router is not appending the search term in the address line correctly.

[christianlupus]

The Abort Edit button has an eye as icon

Yes, this is true and was also the case with the older versions. The idea was a to indicate a view-only mode.

[seyfeb]

I cannot reproduce. What do you mean by distorted?

Exactly what you see in the picture. Compare to the original photo, the plate should be round.

[seyfeb]

The Abort Edit button has an eye as icon

Yes, this is true and was also the case with the older versions. The idea was a to indicate a view-only mode.

I see. I'm not sure anyone would understand or expect this for an "abort" action without the explanation, but this is merely a cosmetic issue.

[christianlupus]

I cannot reproduce. What do you mean by distorted?

Exactly what you see in the picture. Compare to the original photo, the plate should be round.

I fear this is part of the upstream JSON data. It is (manually extracted from the URL)

{
  "@context": "http://schema.org/",
  "@type": "Recipe",
  "name": "Pizzetta Caprese mit Mozzarella Antipasti und Rucola",
  "author": "HelloFresh",
  "image": "https://img.hellofresh.com/f_auto,fl_lossy,h_640,q_auto,w_1200/hellofresh_s3/image/pizzetta-caprese-mit-mozzarella-14b69b51-836c7984.jpg",
  "thumbnailUrl": "https://img.hellofresh.com/f_auto,fl_lossy,h_300,q_auto,w_450/hellofresh_s3/image/pizzetta-caprese-mit-mozzarella-14b69b51-836c7984.jpg",
  "description": "Ruck, zuck zubereitet! Mit unseren schnellen Lunch-Rezepten sparst Du mittags Zeit, ohne dabei auf abwechslungsreiche Mahlzeiten zu verzichten. In max. 20 Min. zauberst Du leckere Gerichte und genießt im Handumdrehen einen perfekten Lunch.",
  "datePublished": "2023-10-13T11:03:48+00:00",
  "totalTime": "PT20M",
  "nutrition": {
    "@type": "NutritionInformation",
    "calories": "769 kcal",
    "fatContent": "39.8 g",
    "saturatedFatContent": "15.6 g",
    "carbohydrateContent": "50.7 g",
    "sugarContent": "12.1 g",
    "proteinContent": "26 g",
    "fiberContent": null,
    "cholesterolContent": null,
    "sodiumContent": "2.68 g",
    "servingSize": "355"
  },
  "recipeInstructions": [
    {
      "@type": "HowToStep",
      "text": "Zwiebel halbieren und in feine Streifen schneiden.\nPaprika halbieren, entkernen und in 1 cm Streifen schneiden.\nIn einer großen Pfanne 1 EL [1,5 EL | 2 EL] Olivenöl* erhitzen. Zwiebel- und Paprikastreifen darin 4 – 5 Min. anbraten. \n„Hello Buon Appetito“ hinzugeben und für 30 weitere Sek. anbraten. Mit Salz* und Pfeffer* abschmecken. Herausnehmen und Pfanne auswischen. \nIn der Zwischenzeit mit dem Rezept fortfahren."
    },
    {
      "@type": "HowToStep",
      "text": "In einer großen Schüssel Balsamicocreme, 1 EL [1,5 EL | 2 EL] Olivenöl*, Salz* und Pfeffer* zu einem Dressing vermengen.\nRucola grob hacken und mit dem Dressing vermengen.\nMozzarella in grobe Stücke zupfen.\nTomatenpesto gleichmäßig auf 2 [3 | 4] libanesischen Fladenbroten verteilen. \n "
    },
    {
      "@type": "HowToStep",
      "text": "In 2 Pfannen jeweils 0,5 EL [0,75 EL | 1 EL] Öl* bei mittlerer Temperatur erhitzen. \nFladenbrot hineingeben, Mozzarella darüber verteilen, mit dem Paprika-Gemüse und Hartkäseflakes toppen und abgedeckt für 1 – 2 Min. braten, bis der Boden knusprig und der Käse geschmolzen ist.\nTipp: Du kannst die Pizzetta auch nacheinander in derselben Pfanne braten.\nPizzetta nach Belieben in Stücke schneiden. Mit Rucola und der Basilikumpaste toppen. \nGuten Appetit!"
    }
  ],
  "recipeIngredient": [
    "120 g Libanesisches Fladenbrot",
    "75 g Tomatenpesto",
    "125 g Mozzarella",
    "50 g Rucola",
    "1 Stück Paprika multicolor",
    "1 Stück rote Zwiebel",
    "15 ml Basilikumpaste",
    "12 g Balsamicocreme",
    "4 g Gewürzmischung „Hello Buon Appetito“",
    "20 g Hartkäse ital. Art, geraspelt",
    "nach Geschmack Pfeffer",
    "2 Esslöffel Olivenöl",
    "1 Esslöffel Öl",
    "nach Geschmack Salz"
  ],
  "recipeYield": 2,
  "keywords": [
    "Vegetarisch",
    "Zeit Sparen"
  ],
  "recipeCategory": "Hauptgericht",
  "recipeCuisine": "Italienisch"
}

If I download the linked image, it is said distorted image as in the cookbook app.

[seyfeb]

If I download the linked image, it is said distorted image as in the cookbook app.

Oh o_O Did not expect this. Well, then we can ignore this point!

[christianlupus]

The value for cooking, prep, and total time are not filled in the respective text input fields

This is true also for NC 27. So, this is a previous existing bug and should be handled.

[christianlupus]

Weird cursor in Select component after selecting a value

I cannot check in NC27, as it was not working there as expected (no nutrition shown in FF on my dev instance right now)

Maybe you want to make the selection field unsearchable (searchable="false")?

[christianlupus]

Unclear what ---- option is supposed to mean => Accidentially selecting it deletes line

If it is supposed to delete a line, my suggestion would be to better remove this option (which is not an actual option) and add a trash can symbol next to the line instead.

Yes, it is way to remove an entry from the list of nutrition data. So, yes, it should remove the line.

Adding a trash can might be better in UI but I wanted to avoid doing more changes to the components if possible.

[christianlupus]

I find it confusing that the order of rows changes when an the option of an already filled row in the middle is changed. This looks like the row is deleted while it is only moved to the end of the list for no apparant reason

I wanted to avoid the sorting to be random in the list of nutrition. Thus, I used the same ordering as the props delivered to the component.

We could avoid and just risk that the ordering gets completely messy or store the ordering during editing locally in the component. The alternative would be a transition group that will have a visual resorting effect to make the user aware of the resorted nutrition.

[christianlupus]

All mentioned concerns have been either addressed in their own PR/issue or were addressed directly in this PR (small bugs). Did I miss anything?

Apart from that, I suggest to check this onces more. If it looks rather ok (ignoring the opened issues), we can merge this PR, fix the issues and push out the 0.11.0 ASAP. Of course, if there are more concerns, feel free to communicate them, ideally in separate issues to tackle one after the other.

[seyfeb]

Apart from the fact that we might want to consider showing a progress indicator when a category is being renamed (first, I thought it was not working), I did not notice anything worth mentioning in addition to the issues above. However, I'm going to open an issue for this so we can (finally 🎉 ) release 0.11.0