Skip to content
OverviewSolutions
4
Comments
9
Antonio Aurello
@antonADev

All comments

  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Olaf... First of all congratulations for finishing the challenge 🎉. The overall work is pretty amazing and you've did a great job, there are just some minor design differences:

    • As for your ul element, you don't need a role attribute for it, it's a semantic element and since roles are used to provide a structural description for a section of content, most of these roles should no longer be used as browsers now support semantic HTML element with the same meaning

    • As for the design, like I've said, your work is pretty amazing. Just check some colors of the given design and your project will be perfect. Impressive job done.

    Again, it's an amazing try, this challange is not easy at all and you have to be proud of your achievement.

    Hope my answer will help you to improve your skills and keep coding 😊

    Marked as helpful
    0
  • AMINA MADAT•230
    @aminaNBASubmitted about 2 years ago

    I used HTML 5 / SCSS / JS in toggle button the website is responsive any insight on this, it would be great.

    Responsive News-homepage

    2
    Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Amina... First of all congratulations for finishing the challenge 🎉. The overall work is not bad on desktop size, but there are some issues and the mobile-layout isn't working at all, so I will try to give you some advices:

    • It's best practice to use HTML5 semantic elements for accessibility reasons. Semantic HTML elements are those that clearly describe their meaning in a human- and machine-readable way. Elements such as <header>, <footer> and <article> are all considered semantic because they accurately describe the purpose of the element and the type of content that is inside them. Like those below
    <body> 
     <header>HEADER CONTENT</header> 
     <main>MAIN BODY CONTENT</main> 
     <footer>FOOTER CONTENT</footer> 
</body>

    So I would suggest to refactor your code and use the semantic tags. You could also use divs giving them a role attribute, that is perfectly fine. But a semantic element like your <nav> doesn't need a role attribute, but if you use a div as a landmark purpose, then you have to use the role attribute.

    • As for the design, even if on desktop it doesn't look bad, the mobile-layout is missing completely. I've took a look at your code and you've used media queries, and thats amazing. Go inspect your site and with the existing media queries, just making some changes, you will achieve it.

    • Check for your font styling, because you're using only the regular variant.

    • The navbar logo isn't showing, check your src attribute.

    Overall it's an amazing try, this challange is not easy at all and you have to be proud of your achievement.

    Hope my answer will help you to improve your skills and keep coding 😊

    1
  • Ian•140
    @IanMcbullSubmitted about 2 years ago

    I'd like some feedback on my JS. I feel that I can improve on it and write more DRY code.

    IP Address Tracker

    2
    Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Ian... First of all congratulations for finishing the challenge 🎉. The overall work is not bad, but there are some issues:

    • It's important, for accessibility issues, to use landmarks, or special regions on the page to which screen readers and other assistive technologies can jump. By using landmark elements, you can dramatically improve the navigation experience on your site for users of assistive technology. Like in the example above with the use of HTML5 Semantic Elements.
    <body> 
     <header>HEADER CONTENT</header> 
     <main>MAIN BODY CONTENT</main> 
     <footer>FOOTER CONTENT</footer> 
</body>
    • Buttons should have discernible text that clearly describes the destination, purpose, function, or action for screen reader users. You can achieve this with the aria-label attribute.
    • Images should have always an alternate text. So, the alt attribute provides alternative information for an image if a user for some reason cannot view it (because of slow connection, an error in the src attribute, or if the user uses a screen reader).

    As for the design, like I've already said, the overall work is not bad, but there are some parts that can be perfectioned:

    • Try always to work on your challenges or projects with the mobile-first approach. In your case, it isn't mobile responsive.
    • Other issues are just some size proportion issues, challenge yourself to play a bit around with your css, you can achieve it in no time.

    Hope my answer will help you to improve your skills and keep coding 😊

    Marked as helpful
    0
  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Titouan... First of all congratulations for finishing the challenge 🎉. The overall work is pretty amazing, there are just some minor issues:

    • The background color of your solution is a bit darker then the given design. Play a bit around with your css and the given style properties, and you will achieve this in a couple of seconds.
    • It has a couple of size issues, overall width and some styling for your links.

    But again, very impressive work and you have to be proud of yourself.

    Hope my answer will help you to improve your skills and keep coding 😊

    Marked as helpful
    0
  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Chetana... First of all congratulations for finishing the challenge 🎉. The overall work is not bad, but there are some issues:

    • It's important, for accessibility issues, to use landmarks, or special regions on the page to which screen readers and other assistive technologies can jump. By using landmark elements, you can dramatically improve the navigation experience on your site for users of assistive technology. Like in the example above with the use of HTML5 Semantic Elements
    <body> 
     <header>HEADER CONTENT</header> 
     <main>MAIN BODY CONTENT</main> 
     <footer>FOOTER CONTENT</footer> 
</body>
    • I've noticed that you've used the same ID for all of your notification images, but IDs must be unique - the same ID should not be used more than once in the same page. Otherwise, they can interfere with the user agent's (e.g. web browser, assistive technology, search engine) ability to properly parse and interpret the page. Depending on how the IDs are used, this could prevent users with disabilities from being able to access and operate the web page properly.

    As for the design, like I've already said, the overall work is not bad, but there are some parts that can be perfectioned:

    • You've implemented just a mobile version, and even if it looks ok on mobile, on desktop it looks way different. So I would suggest to implement even a desktop version with the use of media queries, clamp() and flex.

    Hope my answer will help you to improve your skills and keep coding 😊

    Marked as helpful
    0
  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Malinga... First of all congratulations for finishing the challenge 🎉. No much to say to your work, very impressive.

    Just some accessibility improvements:

    • Generally, it is a best practice to ensure that the beginning of a page's main content starts with a h1 element, so I would suggest to wrap the “Notifications” inside the div, with an h1 element.
    • Wrap the “attribution” div in a <footer> semantic element.

    Remember, it’s best practice to structure your page with HTML5 reference elements (nav, header,main,footer…)

    Hope my answer will help you to improve your skills and keep coding 😊

    Marked as helpful
    1
  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Kristine... First of all congratulations for finishing the challenge 🎉. The overall work is not bad, but there are some issues:

    • The semantic elements should not be nested.
    <body> 
     <header>HEADER CONTENT</header> 
     <main>MAIN BODY CONTENT</main> 
     <footer>FOOTER CONTENT</footer> 
</body>

    So I would suggest to not nest the <header> element as a child of the <main>.

    • The use of the generic <section> element represents a generic standalone section of a document, which doesn't have a more specific semantic element to represent it. Sections should always have a heading. So I would refactor your code, removing all those sections and maybe just use one section element for all the notifications.

    As for the design, like I've already said, the overall work is not bad, but there are some parts that can be perfectioned, but overall it's very impressive.

    Hope my answer will help you to improve your skills and keep coding 😊

    Marked as helpful
    0
  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Hi Mohaimin... First of all congratulations for finishing the challenge 🎉. The overall work is not bad, but there are some issues:

    • First and foremost, the <main> element represents the dominant content of the <body>, so it should not be a child element of any other element, exept of the <body> element.
    • The use of the generic <section> element represents a generic standalone section of a document, which doesn't have a more specific semantic element to represent it. Sections should always have a heading. So I would refactor your code, removing all those sections, and put, for example, all the notifications in the <main> element.

    As for the design, like I've already said, the overall work is not bad, but there are some parts missing:

    • The unread notifications should be visible on the page load, and only become read on the click of the 'Mark all as read' button. It means that the style should become like the last four comments below, from an unread state to an read state, and not invisible at all.
    • The counter near the Notification header should react on the click and become 0.
    • The commented picture figure is missing in the 'commented on your picture' notification.

    Hope my answer will help you to improve your skills and keep coding 😊

    0
  • Antonio Aurello•230
    @antonADevPosted about 2 years ago

    Your work is very impressive. Just some minor UI differences but still a very good job. The thing I can suggest is that <main> and <header> are intended to be at the same level of hierarchy in the page (one level below <body> ). Nesting one inside the other would therefore not be recommended. But overall, very impressive. Good job :)

    Marked as helpful
    1