Simple-Omlette-Recipe

Community feedback

• 

  T

  Grace•31,090

  @grace-snowPosted 8 months ago

  This doesn't look finished to me. Your solution doesn't fit on mobile, which it should do by default because that's how html works. I haven't looked at the code yet but expect you've used widths when the component only needs a single max width in rem.

  Note also how your font sizes (especially headings) and line heights look much smaller than the design. Try to get it much closer if you can.

  Marked as helpful

  0

  ---

  

  T

  Grace•31,090

  @grace-snowPosted 8 months ago

  Looking at the code:

  1. You don't need to repeat the Google font reconnect links in the html head. In fact you can download all of the actual font families with their weights in one link as well instead of listing the families separately. Only link the weights you actually need too.
  2. Remove all IDs in this and do not use IDs for styling ever. That's not what they're for.
  3. Remember emphasised bold text should be in a strong tag not span.
  4. You must use a table for the tabular data. This table must have appropriate header cells and scope on those set to "row".
  5. The attribution belongs in a footer landmark outside of main.
  6. Always include a modern css reset at the start of the styles in every project. Get into the habit now. Andy Bell or Josh Comeau both have good ones you can look up and use.
  7. You must remove the width from the component and use max-width in rem instead. This is what's causing the bugs I'm seeing.
  8. Font size must never be in px
  9. I think you have incorrect heading levels in this. All of the h3s should be h2s (they are not subheadings of "preparation", they are subheadings of the recipe itself).
  10. Remove the huge margin-lefts. As soon as you find yourself setting large margins to try and create a layout alarm bells should ring in your head. Pause and consider another way once the html is correct for the table.
  11. It's a real shame to remove text-decoration underline from links. It's actually an accessibility failure too under WCAG criteria because you're relying on color alone to make a link look clickable. In short, keep the underlines on anchors!

  Marked as helpful

  0

  

  Amna Nasir•80

  @amna1526Posted 8 months ago

  @grace-snow Thanks a lot. I will do all this, also what are ids for if not specific styling? should i use classes instead?

  0

  

  Amna Nasir•80

  @amna1526Posted 8 months ago

  @grace-snow I used span in Preparation because the text has a different color too...

  0

  

  T

  Grace•31,090

  @grace-snowPosted 8 months ago

  @amna1526 IDs are for referencing between elements. Very important for accessibility and navigation.

  Here is an in depth article on the ID attribute

  Marked as helpful

  0
• 

  Pablo-Lima-Da-Silva•60

  @Pablo-Lima-Da-SilvaPosted 8 months ago

  muito bom, mas para alinhar os elementos use o margin: auto e padding: auto, irá ajudar bastante

  0

  ---

  

  Amna Nasir•80

  @amna1526Posted 8 months ago

  @Pablo-Lima-Da-Silva I'm sorry I do not speak this language...

  0