New thump and full image #418
Conversation
Codecov Report
@@ Coverage Diff @@
## master #418 +/- ##
========================================
Coverage 0.95% 0.95%
Complexity 401 401
========================================
Files 13 13
Lines 1263 1263
========================================
Hits 12 12
Misses 1251 1251
Flags with carried forward coverage won't be shown. Click here to find out more.
|
There was a problem hiding this comment.
Why are two different SVG files required? The idea was to have a single well-scalable image.
Additionally, it might be a good idea to pop things up with CSS+Vue changes so that the existing images are replaced by an SVG image + gray background of appropriate size.
Additionally, you should sign-off the commits to make DCO happy.
|
One SVG makes more Sense. I hadn't thought through to the end. My mistake. I'll improve it. |
|
I will have a look later as I am quite busy currently due to day-time job. |
christianlupus
left a comment
christianlupus
left a comment
There was a problem hiding this comment.
There is a bug in the PR. When opening the cookbook app, I get these errors in the console:
|
I look at this tomorrow. |
Signed-off-by: TheMBeat <[email protected]>
Signed-off-by: TheMBeat <[email protected]>
Signed-off-by: TheMBeat <[email protected]>
Signed-off-by: TheMBeat <[email protected]>
Signed-off-by: Marcel Auclair <[email protected]>
Signed-off-by: Marcel Auclair <[email protected]>
TheMBeat
left a comment
TheMBeat
left a comment
There was a problem hiding this comment.
@christianlupus are the changes ok for you now?
src/components/AppIndex.vue
Outdated
| <li v-for="(recipe, index) in filteredRecipes" :key="recipe.recipe_id" v-show="recipeVisible(index)"> | ||
| <router-link :to="'/recipe/'+recipe.recipe_id"> | ||
| <img v-if="recipe.imageUrl" :src="recipe.imageUrl"> | ||
| <img v-if="$store.state.recipe.image" :src="recipe.imageUrl"> |
There was a problem hiding this comment.
@christianlupus I think the bug comes from this line.
Signed-off-by: Christian Wolf <[email protected]>
Signed-off-by: Christian Wolf <[email protected]>
christianlupus
left a comment
christianlupus
left a comment
There was a problem hiding this comment.
Looks good and I see no issues here.
|
This leads to a lot of problems in my App development. The browser is totally happy if instead of a PNG he receives a SVG. But for normal App Components this always results in crashes deep in the library. As it gets a proper response but cannot parse the response into an image of any type. Would it be ok if i changed the code so that the RecipeController returns 404 if no image is found. And the frontend itself adds the SVG in these cases? This seems a much cleaner "API" solution. |
|
@Teifun2 if this needs more discussion, we might be better of in a new issue instead of this PR. Apart from that, can't you check the |
3 participants
Fix #415