Accessible and Responsive design

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 6 months ago

  It's all the paddings in percentages that are causing problems in the styling. Don't use percentage like that. You lose all control because you don't know what a % will equate to, depending on the end users device, orientation and user agent settings.

  Marked as helpful

  0
• 

  T

  Grace•31,270

  @grace-snowPosted 6 months ago

  This challenge is mainly designed to focus on good html structure. But there are a few issues I note in your solution:

  1. The alt description on the image needs improving. There's a great post in the resources channel on discord about how and when to write alt text.
  2. Strong is the tag for bold emphasis, not span.
  3. Ideally headings should go in order. So preparation ingredients instruction and nutrition should all be h2, not h3.
  4. The table must use heading cells (th) for the row headers, not normal cells (td). Ideally, these header cells should also have a scope attribute set to "row" so it's clear they are headers for the row, not headers for the column.
  5. Font size must never be in px
  6. Get into the habit of including a full modern css reset at the start of the styles in every project. Andy Bell or Josh Comeau both have good ones you can look up and use.
  7. Don't set padding in percentages. You lose all control that way. Instead use px if you want it to be static, or rem if you want it to scale with the users chosen base text size, or em if you want it to scale with the current element's font size. Or if you want to get fancy you could use CSS functions like min(), max(), or clamp() for padding, but I'd say that's unnecessary to learn yet.
  8. The paths are incorrect in html img and the css font declarations. The paths should either start with ./ or with no forward slash at all. It's worth reading up about paths for future reference.

  This solution is rendering extremely narrow on my phone and that is likely caused by the percentage paddings.

  Marked as helpful

  0

  ---

  

  Mohammed Majd•120

  @MajdMohammedPosted 6 months ago

  Hello @grace-snow, I hope you're doing well.

  I am extremely grateful for your tips; your feedback has opened my eyes to numerous tricks to enhance my code and become more professional. I believe I have addressed and implemented all your suggestions, but for table padding, I opted for percentage padding because it enables a responsive table on mobile devices. I attempted using 'rem' and 'pixels', but they don't seem to scale well on very small screens. I would appreciate it if you could explain or assist me with this particular issue.

  Please feel free to re-check my code if you have the time. Once again, thank you so much, @grace-snow. <3

  0

  

  T

  Grace•31,270

  @grace-snowPosted 6 months ago

  @MajdMohammed it doesn't look fixed to me I'm afraid. I can't add a screenshot here but it is extremely narrow with some content spilling out of the component. Make sure you check it on small devices and with enlarged text. Sites must work without any sideways scrolling at 320px wide.

  Marked as helpful

  0

  

  Mohammed Majd•120

  @MajdMohammedPosted 6 months ago

  @grace-snow First of all thanks for your connection.

  I checked it out. I noticed that the size of the header doesn't scale with the page width, I am having also this issue in a new challenge I just started a few hours ago. Here is a link for the latest challenge if you wanna check it out: https://github.com/MajdMohammed/FE-Mentor-Social-links-profile

  I am trying to find some useful articles to read about responsive design especially when it comes to font size. One of the most common solutions I found is adding this line of code: <meta name="viewport" content="width=device-width, initial-scale=1.0">, which I already have :/.

  Edit: PS: I sent a friend request on Discord to share files if needed. Thank you!

  0

  

  T

  Grace•31,270

  @grace-snowPosted 6 months ago

  @MajdMohammed I also notice the html on the table is wrong. You've made them ALL into header cells now. That makes no sense. Look up these elements and make sure you understand the guidance given.

  Marked as helpful

  1
• 

  Mohammed Majd•120

  @MajdMohammedPosted 6 months ago

  Hello @grace-snow,

  Repo is updated:

  • padding percentage replaced with clamp().
  • main font-size is now changed to clamp() to stay flued with the container.
  • table elements are changed to th and td.
  • fixed some slight differences with the elements margin.

  Please re-check the repo when you have the time. Thank you !

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted 6 months ago

  @MajdMohammed the clamp you've added for font size makes this inaccessible I'm afraid. The middle value must not be in viewport units. You need to convert that to rem. So 1vw + 1rem is fine but viewport units only is not.

  You're really confusing things in the way you're using clamp for padding and fit-content for width. I really recommend against these while you're learning, the underlying understanding of the implications is missing. You don't need anything complex in this challenge! The main component literally only needs a max width in rem, that's all. And the body needs a very small padding value. That's all. Keep things simple.

  Marked as helpful

  0

  

  Mohammed Majd•120

  @MajdMohammedPosted 6 months ago

  @grace-snow This is really genius, using 1rem + 1vw will allow the font-size to benefit from rem and vw advtanges. Thank you so much again for this brilliant 💡 idea. What about the response ? Is it good enough ?

  0
• 

  Mohammed Majd•120

  @MajdMohammedPosted 6 months ago

  I think now i completely understand the flaws with using percentages, But if I still remember, when I use rem i also encounter same issue with styling. But I trust your judgement I will never use percentage in padding again !. Next time when i have the opportunity i will fix the font-size plus the table html.

  Have a good day mam.

  0