Vanza Setia• 27,855
@vanzasetia
Posted
Hi There! 👋
Congratulations on finishing this challenge! 👏
I have some feedback on this solution:
- Accessibility
- All the page content should live inside landmark elements (
header,main, andfooter). By using them correctly, users of assistive technology navigate the website easily. In this case, wrap all of it withmaintag.
- All the page content should live inside landmark elements (
<body>
<main>
page content goes here...
</main>
</body>
- The learn more buttons should be links instead of button elements. If this is a real website, the Learn More buttons would navigate the users to a different webpage.
- Next time, if you are going to use
buttonelement, always specify thetypeof thebutton. In this case, set the type of them astype="button". It's going to prevent the button from behaving unexpectedly. - Create a custom
:focus-visiblestyling to any interactive elements (button, links,input,textarea). This will make the users can navigate this website using keyboard (by usingTabkey) easily. - Headings must be in a logical order. Heading tags can be used by users of assistive technology to navigate the website. If headings are not in a logical order, those users might get confused. In this case, replace all the
h3withh2and if you want to remove the Accessibility Issues, you can have asr-onlyh1.
<main>
<h1 class="sr-only">3 column card component</h1>
...the card content goes below the h1
</main>
- For any decorative images, each
imgtag should have emptyalt=""andaria-hidden="true"attributes to make all web assistive technologies such as screen reader ignore those images. In this case, all images are decorative only. - Use
remor sometimesemunit instead ofpx. Usingpxwill not allow the users to control the size of the page based on their needs. - Styling
- To make the card perfectly in the middle of the page, you can make the
bodyelement as a flexbox container.
- To make the card perfectly in the middle of the page, you can make the
/**
* 1. Make the card vertically center and
* allow the body element to grow if needed
*/
body {
display: flex;
align-items: center;
justify-content: center;
min-height: 100vh; /* 1 */
}
- Add the
borderto the initial state of the Learn More buttons to prevent jumping layout. Also, just make thebackground: transparent;to create the:hovereffect.
That's it! Hope you find this useful! 😁
Marked as helpful
1
Miguel Silva• 510
@migsilva89
Posted
@vanzasetia Thank you for all the tips, I have made some changes and updated the version.
Still cant figure it out how to make the h1 the same as the requested design.
Any tips?
regards, Miguel Silva
0
Vanza Setia• 27,855
@vanzasetia
Posted
@migsilva89 It looks like you haven't imported the font families. I don't see any link in the HTML or @import in the CSS file.
Marked as helpful
0
Miguel Silva• 510
@migsilva89
Posted
@vanzasetia Ops, noob mistake lol.
Thanks for the help!! :)
0
