Skip to content
Submitted almost 3 years ago

3-column preview card component using grid

Roshan K.C•150
@roshankcpkr
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...3-column preview card component
  • HTML
  • CSS
1newbie

Design comparison

SolutionDesign

Solution retrospective

Hoping for suggestions to improve.

Community feedback

  • PhoenixDev22•16,950
    @PhoenixDev22Posted almost 3 years ago

    Hello @roshankcpkr ,

    I have some suggestions regarding your solution:

    • There should be two landmark components as children of the body element - a main (which will be the whole component that wrap the three cards. ) and a footer (which will be the attribution).<Footer> should be in the <main >read more about A simplified web page, might look something like this:. No need for the extra container

    • To tackle the accessibility issues, you can add a <h1> with class="sr-only" (Hidden visually, but present for assistive tech). `.

    .sr-only {
	border: 0 !important;
	clip: rect(1px, 1px, 1px, 1px) !important;
	-webkit-clip-path: inset(50%) !important;
		clip-path: inset(50%) !important; 
	height: 1px !important;
	margin: -1px !important;
	overflow: hidden !important;
	padding: 0 !important;
	position: absolute !important;
	width: 1px !important;
	white-space: nowrap !important;            
}
    • For any decorative images, each img tag should have empty alt="" and aria-hidden="true" attributes to make all web assistive technologies such as screen reader ignore those images inthis challenge, all the image sare all decorative. <img class="icon-img" src="images/icon-sedans.svg" alt="" aria-hidden=``true``>

    CSS

    • border-radius and overflow hidden to the container that wraps the three cards.

    • I recommend to use em and rem units .Both em and rem are flexible, Using px won't allow the user to control the font size based on their needs.

    • To center the card on the middle of the page , you can use the flexbox properties and min-height: 100vh for the ``<body>`like this:

    .container {
    /* grid-template-columns: repeat(3, 370px); */
    /* grid-auto-rows: 500px; */Let the content inside the card element dictate the height of it.
    max-width: 920px;/ instead of setting widths in this, consider using max-width. That will let the component grow up to a point and be limited.*/
    grid-template-columns: repeat(3, 1fr);

    Overall , your solution is good . Hopefully this feedback helps

    Marked as helpful
    1
  • Fluffy Kas•7,715
    @FluffyKasPosted almost 3 years ago

    Hiya,

    Your solution looks great. I only have 2 small suggestions where you could perhaps improve this:

    1. I'd remove the alt texts. They're decorative only, they don't add much to the content. It's best if screen readers just skip them. You could add aria-hidden=true on them, as well.

    2. At the moment, your buttons sort of jump around a bit when hovered, as their height is changing because of the borders added on hover state. You could add border: 2px solid transparent when they're unhovered, this invisible border would solve this issue ^^

    Other than this, it's a neat solution, well done!

    Marked as helpful
    0
    Roshan K.C•150
    @roshankcpkrPosted almost 3 years ago

    @FluffyKas Thank you so much for the suggestions.

    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