Bookmark landing page with flexbox and vanilla js
Design comparison
Solution retrospective
Any feedback is welcome.
Community feedback
- @PhoenixDev22Posted about 2 years ago
Hi Zajac,
Congratulation on completing another FM challenge, I have some suggestions regarding your solution:
HTML
- In the logo , the SVG does not contain any visible text that describes the graphic, we need to add the alternative text (invisible) by:
<title>
A short title of the SVG</title>
- Add the appropriate ID’s to the
<title>
. - It must be the first child of its parent element On the <svg> tag, add aria-labelledby="uniqueTitleID”. The same for the footer’s logo.
1- The button
class="hamburger"
needs to have anaria-label
attribute or ansr-only
text that describes the button purpose.` For example, you can have: aria-label='Mobile Navigation Trigger' or 'Open Menu.’2- Adding
aria-expanded
that, the user will be able to know that the button content is expanded or collapsed. At first, it has the “false” as a value then you use JavaScript to change the value.3- You should use
aria-controls
attribute on the toggle element, it should reference theid
value of the ``<ul> ```element.- Avoid creating duplicate content (duplicate navigation). You can style the same navigation in mobile and desktop differently using media queries. Practice like this can result in a poor user experience, when a visitor finds substantially the same content repeated within a set of search results.
- look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content For decorative images, you set an empty
alt
to it with anaria-hidden=”true”
to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images for examplebg-dots, social svgs, ..
- The alternate text for the Google, Firefox and Opera are needed. As they are informative images, you can use the company’s name.
- You might already be aware of this, but you can use the native HTML
details
andsummary
elements to quite easily make interactive accordions that are fairly accessible.
- In
btn-card, btn-small
A button with no type attribute acts as type=”submit”, and will attempt to submit form data when clicked. Be explicit in your intentions and kind to future developers working with your code: provide a type. By specifying either button, submit or reset, the code’s purpose is clear and is easier to maintain
- Forms with proper inputs and labels are much easier for people to use. To pair the label and input, one way is an explicit label’s
for
attribute value must match its input’sid
value. Input fields without accompanying labels can lead to accessibility issues for those who rely on screen readers. If a screen reader comes across an input field without a label it will try to find some accompanying text to use as the label. (To hide the label visually but present for assistive technology, you may usesr-only
class ).
- Instead of using a generic div to wrap the social links , you put your links within an unordered list structure <ul> so that a screen reader will read out how many things are in the list to give visually impaired users the most information possible about the contents of the navigation.
-
In this instance social icons, they do not navigate the user anywhere (they are not clickable). You should use interactive element
<a>
, for example:<li><a href=”#”><svg aria-hidden=”true” focusable=”false”>...</svg></a></li>
-
Links must have discernible text. The social links wrapping the icons must have
aria-label
orsr-only
text indicate where the link will take the user. Within them the svgs, they are purely decorative, you'll need to manually add an aria-hidden attribute and focusable=”false” to each of your svgs.
[You may like to check my solution to the same challenge, it might help and if you have any recommendation please leave a comment .] (https://www.frontendmentor.io/solutions/bookmark-landing-page-mobile-first-lSSOu4P-bg)
Aside these, Great job on this one . Hopefully this feedback helps.
-
Never use
<div>
alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together.* By adding semantic tags to your document, you provide additional information about the document, which aids in communication. -
For the reason stated before, instead of using a generic
div
in<div class="cta__contact-us">
, use<form>
-
The social links wrapping the icons must have
aria-label
orsr-only
text indicate where the link will take the user. Then you setaria-hidden =”true”
to the icons to be ignored by assistive technology.
Aside these, you did great work. Hopefully this feedback helps.
Marked as helpful1 - @flamssdevPosted about 2 years ago
Hey Zajac, I'm just starting and I was having trouble creating an FAQ, you helped me a lot with your clean code, you made me not give up.
1
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