Skip to content
Submitted about 2 months ago

Four card feature section solution

Untest57•80
@Untest57
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Four card feature section
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Community feedback

  • py-code314•340
    @py-code314Posted about 2 months ago

    Hello,

    nice job with the project. Thought I could give you some suggestions regarding your code.

    1. I noticed that your /src folder contains all the projects. Aren't we supposed to make separate repos for different projects?
    2. It's better not to use <br> in h1. Please refer to this MDN article . Instead you can use display: block on <span>
    3. Is there a reason you used two different methods when defining the colors in colors.scss?
    4. I think rather than limiting the width of <body>, better approach would be using justify-content: center. You also don't need margin: 0 auto
    5. Text in .header__description taking up whole width of the screen when screen size goes below 768px. I think it's a bug and you can use max-width in ch units to fix that
    6. It should be 4 rows in the grid with height - auto on them in desktop view
    7. We shouldn't give fixed width and height to cards. Width should be defined in grid-template-columns property and contents of the card should determine the height
    8. .cyan & .blue should be occupying middle 2 rows, .red should be occupying top 2 rows and .orange bottom 2. I can't figure out how 'cyan' & 'blue' sitting in the middle like that lol
    9. Border radius can be increased to match the curve in design

    These are just suggestions and hopefully helpful. All the best

    Marked as helpful
    1
    Untest57•80
    @Untest57Posted about 2 months ago

    @py-code314 Thank you for your feedback. I have applied your suggestions to the code and will provide you with an answer.

    1. I didn't want the repository to increase while doing the 'Frontend Mentor' challenge. And I wanted to easily distribute it to one domain, so I used one repository.

    2. Thank you for the great suggestion. The reason I wrote <br> was because I didn’t think of the method you suggested.

    3. When trying to match the border and the given color, I tried to use a css variable and could not fix it after switching to the scss function.

    4. I modified it according to your opinion, but since <body> itself needs to be centered, margin: 0 auto should be used.

    5. This was intentional. During responsive work, if the .header__description part is not wide enough, it looks too empty, so I did not specify the left and right sizes.

    6. I did not understand the opinion you suggested.

    7. The reason I specified the size directly on the card was because I wanted the size of the card to be fixed.

    8. I also couldn't read it just by looking at the CSS, so I added grid-column.

    9. It was applied later and was not applied to the screenshot. I created a screenshot again.

    Thanks again for your feedback.

    0

Please log in to post a comment

Log in with GitHub

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord