3-column-preview-card using HTML and CSS only

Community feedback

• 

  

  Grace•32,130

  @grace-snow

  Posted almost 3 years ago

  Here are some suggestions for improvements

  • footer does not go inside main. They are separate landmarks
  • alt on the icons can be left blank - these are decorative
  • it's unusual to have 3 h1s on a page. As this is a component that would likely sit on a fuller page, it would be more likely the headings would need to be h2s.
  • I don't think a list is necessary for the markup of these cards. If in a plain document you wouldn't have all this content in a bullets list so there is no need to here. The semantic structure from the headings on each card is fine. If use divs
  • you are setting the font size smaller than the style guide says. 0.8rem is equivalent to 12.8px (very small!) but the style guide says 15px. That means you should be using 0.9375rem
  • Never style on IDs. It is not what they are for and you are increasing specificity for no reason. It can only cause problems
  • margin does not need to be large on the component
  • you can set border radius on the whole component along with overflow hidden. Shorter and means yiu don't need to set it on individual card corners
  • the "Active" states in the design are actually communicating what should happen on hover. If you apply them on :active like that it will create a flash of the color change. You can set active and hover to be the same style if you want though
  • interactive elements are all missing :focus-visible styles. These are extremely important for keyboard users. Choose something bold/obvious - usually a bold outline is what's used
  • I wouldn't position buttons absolutely like that. Instead, consider making each card a flex column. That would allow you to place margin top auto on the buttons and push them to the bottom of each card
  • consider adding a transition property to buttons for smoother color changing
  • rather than changing the colors on each button, there is actually a blend mode you can use that would invert the text and background automatically for all of them. Just an idea

  42

  Marked as helpful
• 

  Travolgi 🍕•31,320

  @denielden

  Posted almost 3 years ago

  Hey Lawal, congratulations on completing the challenge! You did a very great job 😉

  Tip of graphic design: with font-family:" Big Shoulders Display ", cursive the browser will use the Comics Sans font when it doesn't find the first font indicated (you can seen during loading)... for the designer it's a really awful font! I would rather replace it with a font-family:" Big Shoulders Display ", sans-serif much more similar to the primary font.

  Hope this help! Happy coding 😁

  20

  Marked as helpful
• 

  Vanza Setia•27,715

  @vanzasetia

  Posted almost 3 years ago

  Hi, Hamzat! 👋

  I get the chance to online today, so here is some feedback from me.

  • Use CSS to uppercase the text. The uppercased word in the HTML might be spelled by the screen reader (spelled letter by letter).
  • Remove the commented code. If another developer (it can be you in the future) wants to improve this solution, he/she might get confused about whether or not the commented code should be used or deleted.
  • I recommend changing the attribution font size to rem instead of px.
  • I suggest adding width and height attributes to each img element. It is used to create a placeholder while the images are not fully loaded. As a result, it prevents the jumping layout (decreasing the Cumulative Layout Shift). If you want to learn more about CLS, I recommend reading the article from web.dev about it.

  Overall, you've done great work on improving your solution based on the previous feedback. Good job with it! 👏

  That's it! Hope you find this useful!

  17

  Marked as helpful
• 

  Hamzat Lawal•560

  @EngineerHamziey

  Posted almost 3 years ago

  Please I forgot to ask What do they mean by "the design sis created in the following widths: -Mobile: 375px -Desktop: 1440px" Are they referring to media queries or max-width ?

  01
• 

  Hamzat Lawal•560

  @EngineerHamziey

  Posted almost 3 years ago

  One of the problem I'm facing is , I'm using latitude 2100 😩 That has a width of 10.1 inches 1024px...I'll get another one soon, hopefully 😊 I'm disappointed to see that my design is so small than the actual design.

  @vanzasetia @grace-snow... I'll love to hear your review.. I always like to hear from you guys.. I've learnt alot from your comments and solution

  00