Made with css grid (and added a small animation for the first time!)

Community feedback

• 

  P

  solvman•1,650

  @solvmanPosted 9 months ago

  Hi @JanAbe 👋

  Very well done! 👍 Congratulations! 🎉

  I have a few minor suggestions that might help you to improve your project:

  • ⭐️ Consider swapping background color with text/button color. The background color should be lighter than the text/button color.
  • ⭐️ There is a minor clipping issue when the width is 670px to 600px. That is caused by the fact that there is no space left to shrink the width anymore due to the fixed button size and padding/margin around it. There are a couple of ways you can address it. One way is to add min-width (it will add an annoying scroll bar at the bottom). Or you could make your whole layout jump to mobile at 670px vs 600px.
  • ⭐️ There is a button shifting issue on the widths of 702px-713px, 818px-794px, and 990px-950px. That is caused by the hard-set margin-top on your button element. Perpahs, consider making each card a flex/grid container and adding align-self: flex-end to the button class to make buttons "stick" to the bottom of the container.

  Otherwise, great job! 🎊 I hope you find my comments useful 🫶

  Marked as helpful

  1

  ---

  

  JanAbe•230

  @JanAbePosted 9 months ago

  @solvman

  Thank you so much for your comment and your time! It is highly appreciated.

  • I added a min-width to the button even as a max-width. Now there is no clipping issue anymore!

  • I fixed the button shifting issue by doing what you suggested. I added a display: grid to the container and a align-self: center to the children.

  But i don't really understand your first point? Which colours should I swap? I don't understand, sorry.

  Also, how did you find these mistakes? Just by looking meticulously at the website and trying different widths? Because I didn't even know these bugs were present haha.

  again, thank you so much.

  1

  

  P

  solvman•1,650

  @solvmanPosted 9 months ago

  @JanAbe

  Your body background-color: lightgray; should be white, and bottons' background-color: white and wrapper class color: white; should be lightgrey.

  Also, it's a tiny little challenge for you. You might notice that the color of each button's text in the original design should represent the background color of the card it is located on, e.g., an orange card with orange text on its button, a cyan card with cyan text on its button, and so on. You have all button text as cyan. See if you can implement that, too. 😁

  As far as my approach to finding this improvement:

  • ⭐️ I briefly review HTML structure and CSS files to see if they conform to standards to the best of my abilities 😅
  • ⭐️ I try to test different widths and heights by slowly resizing the window and paying attention to how each element responds.
  • ⭐️ I only comment on projects that I have already completed myself. I try to recall what was challenging for me and check how others address the issue. If I find that my solution is lacking, I compliment the person and improve my solution. If it is the other way around, I suggest improvement.

  Marked as helpful

  1