Skip to content
OverviewSolutions
23
Comments
71

Yemisrach

@Yemisrach151,430 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

I’m currently learning...

Accessibility and ReactJS

Latest solutions

View more solutions

Latest comments

  • Yemisrach•1,430
    @Yemisrach15

    Hi Alfred, your solution is really good to be honest. You've used semantic HTML and the class names are also good. A few suggestions I can give you,

    • It needs some tweaking. Try to adjust the margins, padding, ...etc for different screen sizes so that it looks similar to the design. Use the mobile-first workflow.
    • For images, you should have an alternative text in the alt attribute. For instance, for the logo, it could be
    <img src="images/logo.svg" alt="Huddle Company" class="logo">

    or such descriptive phrase. If the image is for decoration only though, you don't need to have an alt text.

    • The .call-out can be improved like the below snippet.
    *html*
<div class="call--out">
    <h2 class="call-out-heading">ready to build your community</h2>
    <a href="#" class="btn btn--primary">get started for free</a>
</div>

*css*
.call-out {
    /* other styles you had */
    display: flex;
    flex-direction: column;
    align-items: center;
    justify-content: center;
}

.call-out-heading {
    /* other styles you had */
    text-align: center;
}

    Notice that I removed the .call-out__header div since it doesn't have any use and is only cluttering the document.

  • Yemisrach•1,430
    @Yemisrach15

    Hi Rashid, Well done! I like your solution. At the moment, the HTML semantics of the bar chart is not that good for accessibility. Using table is better in my opinion.

    PS. I used vercel and didn't have to configure anything. It detects your framework and bundler :)

    Marked as helpful
  • Yemisrach•1,430
    @Yemisrach15

    Hi Noor, I like that you did this from scratch. A few suggestions,

    • A little work is needed on the responsiveness for smaller mobile views.
    • One h1 per page. You could wrap the spending - 7 days header with an h2.
  • Yemisrach•1,430
    @Yemisrach15

    Hi Felipe, responsiveness is well done. Few things I suggest you do,

    • You should include the overview section inside main
    • Change the text dark mode to light mode or vice versa when checkbox is toggled.
    • Input elements should have an associated label with discernible text for visually impaired users. Have a sr-only (screen reader only) text hidden from view but available to screen readers. The text could be something like Toggle mode.
    • The social media icons here have meanings and are not only for decoration so, they should have non-empty alt attributes that describe them.

    Congrats! No bugs in the method for dark mode toggle.

    Marked as helpful
  • Yemisrach•1,430
    @Yemisrach15

    Hey David, solution looks pretty responsive :) A few suggestion I think would help you,

    • For the images inside the header, you can use the picture element to provide alternate images for difference device sizes. Here's a link for reference. Basically, what you have to do is as follow
    <picture>
  <source srcset="<link to desktop image>" media="(min-width: 768px)" />
  <img src="<link to mobile image>" />
</picture>
    • The remove (x) icon besides the tags should be a button since they're clickable. Buttons have the ability to be focused by keyboard users while the icon here is simply an image and there's no way of knowing whether it's interactive.
    • Same goes for the clear element/button and the tags in the cards.
    • For the alt attribute of images, you should provide a text that describes the image well for visually impaired users. So instead of a simple Company logo, you can be more specific and say, for instance, Photosnap's Logo.

    Hope these are helpful to you :)

    Marked as helpful
  • Yemisrach•1,430
    @Yemisrach15

    Hey Nadia, hooray for the zero report! The sass architecture is useful as they say. I think it's more maintainable if you have a separate file for each component. I'm also trying to use 7-1 pattern. Here are a couple of suggestions I can give you,

    • Put the nav element outside main and inside a header element.
    • I wouldn't use article for the element with class .card. articles should have self-contained information but here the information is too large, in my opinion. For the blocks/boxes though it might be appropriate.
View more comments