All comments

• 

  Emin Muratoglu•190

  @scorpion61Submitted about 4 years ago

  Tried to implement the css Flexbox and grid layout on this project. You are welcome to review. Thanks.

  

  Responsive page using CSS grid and Flexbox

  1

  30
  

  sonickonic•700

  @sonickonicPosted about 4 years ago

  Hey 👋 Congrats on submitting your first challenge! Nice work the :hover effect is super cool! 🚀

  On a widescreen, the cards are extremely wide did you consider max-width property?

  Don't forget to correct the HTML ISSUES, they are easy to solve)

  1

  This comment has received 1 replyShow
• 

  Javier Labrador•95

  @JALCH-1512Submitted about 4 years ago

  I think my code is more complex than it should be. I would appreciate any advice to improve it, as I would also like you to tell me what good practices I can implement in my code. :)

  

  Single price grid component with CSS Grid and Flexbox.

  1

  30
  

  sonickonic•700

  @sonickonicPosted about 4 years ago

  Hey Javier! Nice work the screenshot looks awesome! 😎

  Your code looks good overall, I have just a few observations:

  1. You've currently got multiple <p> elements, where different HTML elements may be more appropriate.

  • The <p> defines a paragraph. The best use for it will be in the "Join our community" section, instead of the <div>.
  • In the "Why Us" section, instead of using <p> twice, a <h2> for the heading and <ul> for the list of features will suit better for semantic and accessibility

  2. CSS reset will set the whole page to 0, for a fresh clean start, instead of setting margin:0; to a specific element.

  * { margin:0; padding:0; }

  Have you tried a Mobile-first approach? It's quite a common workflow, you starting with the mobile version and switch to min-width media queries instead of max-width. It helps to simplify the CSS code)

  1

  This comment has received 2 repliesShow
• 

  Erick Vargas•150

  @eriandevSubmitted over 4 years ago

  

  Pricing component with tailwindCSS

  1

  10
  

  sonickonic•700

  @sonickonicPosted over 4 years ago

  Nice work, it looks great and scales down really good to mobile. Well done! 👍

  0
• 

  Aayush Srivastava•65

  @aayush182Submitted over 4 years ago

  Please give feedback on my solution.

  

  Four card feature section

  1

  00
  

  sonickonic•700

  @sonickonicPosted over 4 years ago

  Congrats on submitting your first solution! it looks amazing!

  You've currently got two h1 elements, it will be better to use it once as it can cause accessibility issues. Two ways to solve it:

  1. Wrap all of that text as a single h1 and use a span element inside it to separate the text.
  2. Use h1 for the main heading and h2 for the sub-title.

  Although it is a pixel perfect solution for the wide-screen, the mobile version is slightly off. Try next time to use min-width media queries instead of max-width. Starting from the mobile-first can lead to more simple solution)

  Good job!

  3

  This comment has received 3 repliesShow