Here are some suggestions for improvements
- footer does not go inside main. They are separate landmarks
- alt on the icons can be left blank - these are decorative
- it's unusual to have 3 h1s on a page. As this is a component that would likely sit on a fuller page, it would be more likely the headings would need to be h2s.
- I don't think a list is necessary for the markup of these cards. If in a plain document you wouldn't have all this content in a bullets list so there is no need to here. The semantic structure from the headings on each card is fine. If use divs
- you are setting the font size smaller than the style guide says. 0.8rem is equivalent to 12.8px (very small!) but the style guide says 15px. That means you should be using 0.9375rem
- Never style on IDs. It is not what they are for and you are increasing specificity for no reason. It can only cause problems
- margin does not need to be large on the component
- you can set border radius on the whole component along with overflow hidden. Shorter and means yiu don't need to set it on individual card corners
- the "Active" states in the design are actually communicating what should happen on hover. If you apply them on :active like that it will create a flash of the color change. You can set active and hover to be the same style if you want though
- interactive elements are all missing :focus-visible styles. These are extremely important for keyboard users. Choose something bold/obvious - usually a bold outline is what's used
- I wouldn't position buttons absolutely like that. Instead, consider making each card a flex column. That would allow you to place margin top auto on the buttons and push them to the bottom of each card
- consider adding a transition property to buttons for smoother color changing
- rather than changing the colors on each button, there is actually a blend mode you can use that would invert the text and background automatically for all of them. Just an idea
Marked as helpful
@EngineerHamziey
Posted
@grace-snow @grace-snow Thanks so much for taking your time. *Concerning this "Never style on IDs. It is not what they are for and you are increasing specificity for no reason. It can only cause problems" this is the second time I'll hear this, but I don't understand what "you are increasing specificity for no reason." means, I'll be glad if you have a material/video that you can recommend on that. because what I learnt earlier from some videos was "use ids for styling one thing and use classes for styling many things. And this is the second time I'll hear about the Specificity thing and I still don"t understand.
- I used active because that is what's written in the picture/design given, i don't like the result too.I'll just add :hover to it to make it look nice.
*OMG I was wondering why margin auto isn't working on the button so i had to use float instead, THANKS FOR THE TIP and TRICKS.
*Could you please explain this further or simplify: "interactive elements are all missing :focus-visible styles. These are extremely important for keyboard users. Choose something bold/obvious - usually a bold outline is what's used" and this "you can set border radius on the whole component along with overflow hidden. Shorter and means you don't need to set it on individual card corners"
*I think I'll need to read more on blend-mode later, I don't know it exist and I even tried Using it and it's not working.
*I was thinking jumping to an h2 without using h1 will introduce accessibility problem, thanks for the correction. *placing footer inside the main was a mistake, thanks for noticing
once again thank you so much. don't let this take all of your time...just answer it at your Convenience.
@vanzasetia
Posted
@EngineerHamziey Let me answer some of the questions that you had.
- First, for the specificity, it's best to keep the specificity of the stylesheet as low and flat as possible. High specificity will make your stylesheet hard to maintain. Ideally,
idshould be used for anchoring (not for styling).- High specificity makes stylesheet harder to maintain because of the following reasons.
- It makes it hard to manipulate the styling. CSS is about manipulating. We have to use the
@mediaquery to overwrite the base styling. So, if the base styling has higher specificity than the selector in the@mediaquery, then it won't work. This might be not a big deal on a small project. But, on a large project where the CSS might contain 1000 lines then it's hard to debug when the issue happens. - It's best to make a habit of writing a low specificity stylesheet. This way, you are already getting used to writing a maintainable stylesheet.
- It makes it hard to manipulate the styling. CSS is about manipulating. We have to use the
- High specificity makes stylesheet harder to maintain because of the following reasons.
- Add
:focus-visiblestyling to all anchor tags in this case. Usually, an outline would be great. In general, make sure that the:focus-visibleis clear and not the same as the:hoverstyling. If you want to see an example, you canTabthrough this page and you will see someoutlineon the interactive that you currently focus on.
.btn:focus-visible {
/* some styling */
}
- You can set the
border-radiuson the parent element and then setoverflow: hiddento all the card elements. After that, you can remove all theborder-radiusproperties from the.cardselements.
/**
* 1. it will automatically make the
* children elements obey the
* border-radius of the parent element
*/
.card-container {
border-radius: 0.4rem;
overflow: hidden; /* 1 */
}
Hope this clears everything.
Marked as helpful
