CSS grid

Community feedback

• 

  Travolgi 🍕•31,400

  @denieldenPosted almost 3 years ago

  Hi Vasile, good job!

  To center the elements on the page try using Flexbox, it can help you better: give the flexbox and height properties to the body and remove margin from the main tag.

  Read the guide -> Flexbox

  Note: Flexbox aligns to the size of the parent container.

  You can use the vh measurement for the height... Viewport Height handles the sizing of an element in relation to the height of the browser window.

  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 ;) And keep it up!

  Marked as helpful

  0
• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted almost 3 years ago

  Hello VasileCosmin,

  I have some suggestions:

  • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images.
  • You can add a <h1> with class="sr-only"(Hidden visually, but present for assistive tech).

  .sr-only {
  	border: 0 !important;
  	clip: rect(1px, 1px, 1px, 1px) !important; /* 1 */
  	-webkit-clip-path: inset(50%) !important;
  		clip-path: inset(50%) !important;  /* 2 */
  	height: 1px !important;
  	margin: -1px !important;
  	overflow: hidden !important;
  	padding: 0 !important;
  	position: absolute !important;
  	width: 1px !important;
  	white-space: nowrap !important;            /* 3 */
  }
  

  This fairly modern technique will hide or clip content that does not fit into a 1-pixel visible area. Like off-screen content, it will be visually hidden but still readable by modern screen readers. -You can replace the <h1 > by <h2>. (the heading should uppercase)

  • Swap the buttons for anchor tags. Clicking those "learn more" buttons would trigger navigation not do an action so button elements would not be right.

  • To make the card perfectly in the middle of the page, you can make the< main> element as a flexbox container .

  • border-radius and overflow hidden to the container that wraps all the cards so you don't have to set it to individual corners.

  • Let the content inside the card element dictate the height of it.

  I recommendnot:

  • use pixel for font-size.

  • A general point - try to put classes directly on anything you want to style. Then you won't need as many nested css selectors.

  Hopefully this feedback helps.

  Marked as helpful

  0
• 

  Luka•320

  @lukakavtarraPosted almost 3 years ago

  hey =) looks like text max-width on your solution is more than it's on the given design. i don't see any problems there good work

  Marked as helpful

  0
• 

  darryncodes•6,370

  @darryncodesPosted almost 3 years ago

  Hi Vasile,

  Nice work on this one - well done!

  You could add transition: ease .3s; to your .btn class to make the hover effect slightly smoother.

  Happy coding!

  0