Skip to content
OverviewSolutions
3
Comments
11

Joshua

@minimalsm25 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

Latest comments

  • Joshua•25
    @minimalsmPosted over 3 years ago

    @grace-snow all changes implemented, thanks again :-)

    0
  • Joshua•25
    @minimalsmPosted over 3 years ago

    aria label isn’t valid on a h1

    Realised that after I submitted. I was fooled by this highly upvoted StackOverflow post. I guess it seemed to the most a11y friendly option, but I'll refactor this to use a sr-only class.

    your BEM is close, but you’ve forgotten to use the block class of card on the article

    Yes! Wasn't sure on this (it was one of my questions). Any thoughts as to using / not using BEM when we're dealing with small components like this?

    line height should be unitless, never in px

    Noted, will refactor 😀!

    letter spacing needs to be in em really. Again, px no good coz it can’t scale with the text of sizes/zoom are changed

    Ah good point, thanks!

    never have a max height on elements containing other content. If I had a larger font size or a smaller device, that max height could break the solution for me

    Lessons learned! Seems stylistically this wasn't doing anything anyway 😅 (at least at 'normal' font-size/zoom)

    Thanks so much for your thorough feedback @grace-snow ❤️

    0
  • Joshua•25
    @minimalsmPosted over 3 years ago

    Hey @ VictorDjun, this is a good start 💪

    Some attention needs to be paid to the details here to make this perfect ☺️.

    Suggestions

    • H2 should be dark-blue (#1F314F), not black.
    • The card needs more width (design is 320px)
    • Instead of setting a width on the image (makes it hard to maintain) you could set the image width to 100% but add padding to the container to achieve the border.
    • Tone down the box-shadow a little (or, a lot 😬)
    0
  • Joshua•25
    @minimalsmPosted over 3 years ago

    Hey @ VictorDjun, this is a good start 💪

    Some attention needs to be paid to the details here to make this perfect ☺️.

    Suggestions

    • H2 should be dark-blue (#1F314F), not black.
    • The card needs more width (design is 320px)
    • Instead of setting a width on the image (makes it hard to maintain) you could set the image width to 100% but add padding to the container to achieve the border.
    • Tone down the box-shadow a little (or, a lot 😬)
    Marked as helpful
    0
  • Sj•120
    @SJ-74Submitted over 3 years ago
    What are you most proud of, and what would you do differently next time?

    I used SCSS for the first time in any FEM challenge and would like to improve my SCSS knowledge as of now.

    What challenges did you encounter, and how did you overcome them?

    Nothing much as this was very beginner friendly challenge and I have already completed few challenges before this.

    What specific areas of your project would you like help with?

    Would appreciate feedback on use of SCSS.

    Mobile first using CSS flexbox

    #bem#sass/scss
    3
    Joshua•25
    @minimalsmPosted over 3 years ago

    Looking good @SJ-74 💪!

    Few suggestions to get it closer to the original design:

    • H1 font-size should be bigger (1.375rem)
    • I find setting padding on the bottom of the card instead of the paragraph text gives better (more flexible) control over the spacing.
    1
  • Joshua•25
    @minimalsmPosted over 3 years ago

    Hey @poorneshvishnole, this is a good start 💪

    Some attention needs to be paid to the details here to make this perfect ☺️.

    Suggestions

    • H2 should be dark-blue (#1F314F), not black.
    • H2 should be centered horizontally
    • H2 and P tags should have an explicit font-size set. Currently, they don't match the design.
    • Card container needs more border-radius (Around 16px/1rem)
    Marked as helpful
    1
View more comments