Skip to content
OverviewSolutions
3
Comments
50

Dusan Brankov

@dusan-b860 points

Hi, I'm Dusan. I love writing well-organized code and digging in-depth into various web development and UI/UX design topics. I'm especially interested in web accessibility (a11y) and performance, always looking for sound solutions to provide a great user experience.

Latest solutions

Latest comments

  • Dusan Brankov•860
    @dusan-b

    Hi Leon,

    I've just had a look at your page and code, most of it seems to be quiet reasonable. To answer your question, you should basically use one single <h1> element that describes the most important content of a particular web page. That's especially important for assistive technology users and search engines.

    Regarding accessibility, there are some changes I would consider:

    Adding aria-label="primary" to the <nav> element would make it even clearer to screen reader users that this is the main navigation of the website. Screen readers would announce it as "navigation, primary".

    The "Get Started" button should actually be an <a> element that only by design looks like a button. When a user clicks on it, he/she will certainly be redirected to another web page. A button actually enables users to trigger an action or event, such as submitting a form, opening a dialog etc.

    The titles right after the button seem to belong to the title "All your files in one secure location...", so I think it would make more sense to change them to <h3>.

    As far as the testimonial section is concerned, you could improve the semantics by adding a structure like the following:

    <blockquote>
  <p>... </p>
  <footer>
    <cite>Auhtor name</cite>
    <cite>Job title</cite>
  </footer>
</blockquote>

    The social media icons in the footer represent a group of items, so they should be included in a list, too. Also, you should add a label for each link, otherwise screen readers would announce them just as "link", users would get confused. You can add visually hidden text inside each link element or add aria-label with the appropriate content.

    Besides that, the web page looks pretty good. Keep up the good work. :)

    Marked as helpful
  • Dusan Brankov•860
    @dusan-b

    Hi Leon,

    I just had a look at your code, most of it looks pretty good. There are only a few changes I would consider to improve accessibility.

    Although skipping heading levels should be avoided, I'm not sure if the <h1> heading is appropriate in this case. The card is just a component that would most likely be reused on an existing website. Therefore, <h2> or even <h3> would be more realistic.

    As @tmciesco mentioned, the given alternative text for the image would only cause confusion to screen reader users. You also wouldn't label a link with its URL. Something like "QR code, scan to visit Frontend Mentor" would be more clear about what purpose the image has.

    As far as the HTML structure is concerned, the most of it looks reasonable, but you may want to replace <section> with <article>. MDN makes it very clear, what purpose this element has:

    "The <article> HTML element represents a self-contained composition in a document, page, application, or site, which is intended to be independently distributable or reusable [...]. Examples include: a forum post, a magazine or newspaper article, or a blog entry, a product card, a user-submitted comment, an interactive widget or gadget, or any other independent item of content."

    To be thorough, I would also change the attrbution container to be a <footer> element and take it out of <main>.

    I hope I could help. Keep going, and happy coding. :)

  • Dusan Brankov•860
    @dusan-b

    Hi Jennifer,

    I just had a look at your code, most of it looks pretty good. There are just a few changes I would consider.

    To center the card in both directions, a better way is to use Grid or Flexbox instead of position and transform:

    body {
  display: grid;
  place-content: center;
  min-height: 100vh;
}

    For .card-container it's better to use max-width instead of width as it prevents horizontal scrolling on small screens.

    As far as accessibility is concerned, alternative texts for images should have a clear message. Instead of "QR code", it would be better to have "Scan this QR code to improve your front-end skills".

    Furhtermore, you should avoid skipping heading tags. The most important heading should always be <h1>, the second <h2> etc.

    It's also important to use semantic section elements to allow browsers, assistive software and search engines to understand the structure of your web page. One of the most common ones is the <main> element that contains the main content of a page. Therefore, every web page needs to have a single <main> element. In your case, the card component needs to be wrapped inside <main>. You can easily do this by replacing the parent div tag with main.

    I hope I could help. Keep up the good work, and happy coding. :)

    Marked as helpful
  • Dusan Brankov•860
    @dusan-b

    Hi Valentin,

    To display the background image, you need to change the URL in CSS as follows:

    background-image: url(../images/bg-desktop.svg);

    Besides, the alt attribute of the hero image needs to be empty as long as it's decorative, otherwise screen readers would read it as "hash" (#), which would be confusing.

    The rest looks pretty good. Keep going, and happy coding. :)

    Marked as helpful
  • Dusan Brankov•860
    @dusan-b

    Hi Pierre,

    Anchor elements (<a>) are inline elements, so margin has no effect and the layout breaks. You could also change it to display: inline-block and omit the Flex properties along with max-width: 200px;. The result would be the same.

    I hope I could shed some light on this. Happy coding. :)

    Marked as helpful
  • Dusan Brankov•860
    @dusan-b

    Hi VMH1225,

    To get the hover effect, you should wrap the image like this:

    <a class="overlay" href="#">
  <img class="mainImage" src="images/image-equilibrium.jpg" alt="Equilibrium">
</a>

    And then add the following CSS rules:

    main {
  isolation: isolate;
}

.overlay:hover {
  background: linear-gradient(
    hsla(178, 100%, 50%, 0.5),
    hsla(178, 100%, 50%, 0.5)),
    url(images/icon-view.svg) center no-repeat;
  border-radius: 2%;
}

.mainImage {
  position: relative;
  z-index: -1;
  vertical-align: top;
}

    If something is not clear, feel free to ask. Keep it up, and happy coding. :)

    Marked as helpful
View more comments