Design comparison
Solution retrospective
Updated
Community feedback
- @grace-snowPosted over 1 year ago
Hi, you asked for a11y improvements so here you go
- The logo is wrapped in a link so it is therefore not enough to have only "Fylo" as the image alt as that is currently acting as the link text as well. The link label needs to be "Home - Fylo" and you can do that in several ways. For example you could make that the alt text, or you could use aria-labelledby on the link pointing to the id of a text element with hidden attribute on it that contains the text, or you could use aria-label on the link
- the inputs in this are currently unlabelled. You must label inputs properly with the accessible name for that input
- label elements and button elements should not be wrapped in paragraph elements
- all of the page content should be in landmarks. That includes anywhere you use a section - those need moving inside main
- for testimonials I recommend using figure, figcaption and blockquote
- the footer logo is important content and should have alt
- content like phone numbers and email addresses should really be links
- there is a nav missing from the footer. Once this has been added both navs need labelling eg main and footer
- inside the footer nav should be ONE list not two. You can split one list into multiple columns with css column properties
- social icons are always expected to be clickable so must be accessibly named links
- buttons and inputs must not have explicit widths and heights. They must be able to grow in height in particular. Use max-width and padding instead, never height on elements that contain text content
- I recommend you ensure inputs have a minimum of 1rem font size. Ios will zoom in on focus on any input with a font size less than 16px
- setting width 100vw is very likely to cause horizontal overflow
- check the button font size because text looks extremely small to me
- media queries must always be defined in rem or em not px
- 1200px sounds very late to be making the switch to a large screen layout. Is there not room for the layout to change earlier?
- it looks like there is a lot more than necessary in the media query on this. A lot of that is likely due to using explicit widths and heights when you shouldn't. You want to try and make the css as short and performant as possible. Even things like font size shouldn't have to change.
Marked as helpful0@AmrAbdelgwaadPosted over 1 year ago@grace-snow Thank you so much, I have a couple of questions on these points :
- On point number 3. why I have to?
- On point number 4. I think all my page is surrounded by landmarks I have a header main two sections and a footer there is no content outside these marks does this mean I have to wrap all the page with main ?!!
- On point number 13. You are right it did it before and I fixed it what should I do instead?
- On point number 15. Can I know the reason?
and thank you so much for reading my shitty code I really appreciate it.
0 - @vanzasetiaPosted over 1 year ago
Hi, @AmrAborockpa!
I recommend downloading the necessary icons instead of importing the whole Font Awesome library. This way, your site will load faster.
You can import many font families at once. So, you don't have to use different
<link>
for each font family.For images containing text, make sure the alternative text includes the image's text. In this case, the Fylo logo should have an
alt
value of “Fylo”. Reference: Checklist - The A11Y Project #for-images-containing-text-make-sure-the-alt-description-includes-the-images-textAll the page content should be inside a landmark element. The
<main>
landmark should wrap all the main content (the other<section>
), not just the first section.Wrap each social media icon with an anchor tag.
Prefer unitless numbers for
line-height
values to avoid unexpected results. Learn more — line-height - CSS: Cascading Style Sheets | MDNUse a CSS reset whenever you start a new project. This can help you set the styling foundation easily. My recommendation — A Modern CSS Reset | Andy Bell
I hope this helps. Happy coding! 😄
Marked as helpful0@AmrAbdelgwaadPosted over 1 year ago@vanzasetia thank you for taking the time to review my project but how can i know the line height from px to unitless line hieght
0@vanzasetiaPosted over 1 year ago@AmrAborockpa
I would calculate the
line-height
. For example, if the design says that the font size is16px
and theline-height
is24px
. Then, you need to use the following formula:[line-height value] divided by [font size] = [unitless line-height]
In this example, it would be:
24 / 16 = 1.5
So, you would write
line-height: 1.5
.Marked as helpful0
Please log in to post a comment
Log in with GitHubJoin 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