Closed (fixed)
Project:
Drupal core
Version:
8.7.x-dev
Component:
Umami demo
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
18 Mar 2018 at 09:46 UTC
Updated:
10 Feb 2019 at 00:44 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
kjay commentedAttached is the patch for the updated recipe including recipe image. This patch adds the new recipe and removes the previous version.
Patch needs general review and checking against https://www.drupal.org/project/drupal/issues/2940146
To help with review I've also added a screenshot of the patch as applied to a clean install of 8.6.
Comment #4
kjay commentedThe tests are hard-coded to check whether the first image and first recipe .html file are accessible. The patch in #1 gets rid of the files the tests are trying to test. This patch fixes that.
Comment #5
kjay commented@markconroy pointed out that I had left the baking tin in cm's and not added inches for US English. This has been updated to read "Use a little of the sunflower oil to grease an 8 inch/20cm square baking tin (or similar size)"
Comment #6
eli-tIs there anywhere else we use inches and cm togther? I thought we were currently just using inches.
Comment #7
HAL 9000 commentedHi, I have applied the patch and worked perfectly. Now I see the better recipe and a much improved image as the attached screenshot shows.
Comment #8
alexpottThe only cm in the current text is in the existing brownie recipe. Whereas inch is in the brownie and victoria sponge recipe. Let's only use inches for now/
Comment #9
kjay commentedAttached patch changes to inches only.
Comment #10
cferthorneyReroll of number 9 so it applys cleanly.
Comment #11
eli-tmodules/demo_umami_content/default_content/articles.csv still refers to chocolate-brownie-umami.jpg, but that file has been removed in the patch.
Note this may be changed in #2981351: Write alt text for Umami Baking Mishaps article.
Comment #12
eli-tI also suggest the following amends, based on #2940146: Standardize writing style for Umami in English:
=> 6 tbsp sunflower oil
=> 1 tsp baking powder
=> 6 heaped tsp cocoa powder
I'd consider not specifying heaped and just having more tsp specified. EG 9 tsp cocoa powder has better brevity. We don't use heaped tsp anywhere else.
=> 3 tbsp maple syrup
=> 1/4 tsp sea salt
=> 230ml unsweetened organic soya milk
I don't think this one is actually in the standards, but it is odd to put the adjective after the noun.
Swap cm for inches. Maybe downgrade a few to a couple
Comment #13
Vidushi Mehta commentedHere's a patch with the above mentioned changes by #12.
Comment #15
eli-tThanks @Vidushi Mehta! That addresses everything from #12.
We still have to address the fact that the chocolate-brownie-umami.jpg image was removed here but is still referenced by an article. I'd suggest restoring the image file and referring line in /core/profiles/demo_umami/modules/demo_umami_content/default_content/LICENCE.txt so that this issue doesn't break that article.
Comment #17
Vidushi Mehta commented@Eli-T not getting it can you please explain what needs to be done?
Comment #18
steveparks commentedHi @vidushi-mehta
Reviewing the earlier issues, what @Eli-T was referring to is:
The patch in #2 (and the updated #4) removed the image chocolate-brownie-umami.jpg.
That causes a problem, because the file modules/demo_umami_content/default_content/articles.csv still references that image.
Therefore, please can you update your patch to:
1) reverse this removal - so that the image chocolate-brownie-umami.jpg is still included in Umami
2) reverse the removal of the image credit in /core/profiles/demo_umami/modules/demo_umami_content/default_content/LICENCE.txt
So an updated patch from you would simply be removing these two changes that were included in the patch at #4.
Does that help
Steve
Comment #19
eli-tComment #20
rachel_norfolkI'm working on this at the Global Contribution Weekend in Leeds, UK...
Comment #21
eli-tComment #22
rachel_norfolkComment #23
kjay commentedAnd here is the updated main image for this recipe. This image is already sized and compressed in keeping with the other recipe images.
Comment #24
rachel_norfolkheh - seems patch needs re-rolling first. Oh - and my laptop wouldn't charge... 😂
Comment #25
rachel_norfolkkjay - the references to the image are to vegan-chocolate-nut-brownies.jpg - I'm assuming you want me to change your uploaded file filename at #23 to match?
Comment #26
kjay commented@rachel_norfolk, please do :)
Comment #27
rachel_norfolkOkay - here is a patch, re-rolling #13 and incorporating the feedback in #18 and the new image in #23. re-rolling was "interesting" for some unexplainable (to me) reason.
Comment #28
eli-tWe need to add vegan-chocolate-nut-brownies.jpg to the LICENCE.txt
We are also inconsistent with respect to hyphenization on "ice-cream" in the Summary, but "ice cream" in the Recipe instruction. Wikipedia and I recommend the latter.
Once these are addressed, I think this is good to go.
Comment #29
starshapedMade updates as noted in #28.
Comment #30
markconroy commentedImage added to the licence
Lots of 'ice cream' and no 'ice-cream'
Looks like we are good to go.
(and this unblocks another issue, so double YAY!)
Comment #31
alexpottCommitted and pushed 80c1533b8b to 8.7.x and 14b787e79a to 8.6.x. Thanks!
As an Umami content change backported to 8.6.x.
Fixed file modes on commit. Files need to be 644 not 755.