@shashilo
Posted
Great job Grace. I really enjoyed how thorough you were with accessibility. The H1 was a nice touch. I did find some things that I'd like to highlight:
- The naming convention doesn't follow a true
BEM
naming. There looks to be nested block names inside elements. EX:c-showcase__card-title
- I'm not sure why you're using
EM
units inc-showcase__card-btn
when you already haveREM
set to 16px as the default. This will be a problem if you every nested this component.
And now for the nit picky items:
- The width and height of both desktop and mobile are off. Desktop more than mobile. On Desktop, because your solution is smaller, all of the margins and paddings do not match the design.
- The content inside the card, space between image, title, body, and button are off.
- The button padding is not accurate.
- The card body content has an
opacity
of 0.9 right now, but the design looks much lighter. - I'm not sure what justifies the breakpoint at
680px
. I'm used to using a common standard breakpoint< 768px
which will accommodate for allmobile/tablet
screens from0 to 767px
.
Thanks @shashilo
- The naming follows a component naming system that still uses BEM. The component is treated as the only block
- I intentionally use em for padding on buttons so that it will scale when the font size is changed. This allows for size modifiers that only need to change the font size (
btn—large
) and can improve readability for supe-enlarged and/or zoomed views because the space around the element increases proportionally
With all the others I don’t even try to match specifics on these challenges. If I was pro member I would have and use the figma file to make these accurate, but am not going to worry about that when working from static images 😊
@shashilo
Posted
BEM can be enhanced to whatever you and your team feels is best. I was just mentioning that it didn't follow the proper BEM mythology.
If you want the font to be responsive, you should use clamp() instead of EM. Clamp() is a more stable and modern way to make responsive fonts.
Correct. The goal isn't to be pixel perfect, but it you should have a high level of detail when you implement. I too am not a Pro member, but I can see issues by pulling up the design and implementations side x side. I've worked with several designers and product owners who cared about every single pixel. I was lazy with my implementations early in my career until I was pushed to implement pixel perfect to what the designer had provided. They put their research and skills into designs, it's our job to make them come to life. So they're going to ensure that this process is as precise as possible.