Skip to content
OverviewSolutions
3
Comments
5
Janelle Schuh
@jschuh23

All comments

  • Janelle Schuh•90
    @jschuh23Posted over 2 years ago

    @kayflyS This was a bit of a tough challenge! I have a few suggestions for you :)

    • It looks like none of your images are showing up because the src is not linked properly. If you add a period at the beginning of each of your image references that should do the trick. For instance, `src="./images/icon-star.svg"
    • I would also suggest not using as many div elements and instead adjusting much of your code to use semantic HTML elements such as - header, section, main, footer. Making these changes will clear the accessibility issues from the FEM report. HTML: A good basis for accessibility is a great resource by MDN about semantics for accessibility.

    Hope this is helpful!!

    0
  • Janelle Schuh•90
    @jschuh23Posted over 2 years ago

    @missbaah Great job on the project!! I have yet to work on this one, but plan to jump into the challenge soon :)

    I noticed a few accessibility issues that were identified in the FEM report. Most of these should be easy fixes.

    • All page content must be contained by landmarks The rule description states - "it is best practice to contain all content excepting skip links, within distinct regions such as the header, nav, main, and footer." I would suggest that you either wrap your section class="container" in a main element or change this section element to a main element. That should fix 3 of your accessibility issues.
    • Page should contain a level one heading My suggestion here would be to change your current h2 to an h1 and then resize the h1 in your css. Finally, to follow semantic HTML I would change your div class="attribution" to a footer element.

    Again, great job on this challenge!!! Keep up the good work!

    Marked as helpful
    0
  • Kaio•460
    @kaiohnrSubmitted over 2 years ago

    Can anyone tell me how I can change the color of the social icon in the footer when hovering over it?

    Easybank Landpage

    1
    Janelle Schuh•90
    @jschuh23Posted over 2 years ago

    @kaiohnr The site looks awesome!!

    I did notice that there are a number of accessibility issues per the Frontend Mentor checks.

    • Document should have one main landmark: This can be solved by adding a main element which completely surrounds everything between your header and footer.
    • All page content should be contained by landmarks: I suspect that when you add the main element then this error will go away (?).
    • Links must have discernible text: My suggestion would be to add either alt text such as "Twitter logo" or an aria-label. Then a screenreader would have something to read.

    Regarding the hover color for the social icons....because those are images you cannot use the fill property. That is only for SVGs. So you can either change your icons to SVGs or do something different for your hover.

    Hope this helps :)

    Marked as helpful
    1
  • Abdelghafour•580
    @Abdelghafour122Submitted over 2 years ago

    This is my humble solution Not pixel-perfect

    If anyone has suggestions on how i can improve my code, please let me know, thank you!

    Profile card component

    1
    Janelle Schuh•90
    @jschuh23Posted over 2 years ago

    Great solution @Abdelghafour122! I wouldn't worry too much about pixel perfect as that can be very hard to obtain :)

    It looks like you have some HTML issues per the Frontend Mentor report. One suggestion I would have is to decrease the number of section elements that you're using. Many of these can be changed to a div and that's ok. I might just have one section element that contains the entire card component.

    The MDN website has some great info on the section element. Specifically: If you are only using the element as a styling wrapper, use a div. An unwritten rule is that a section should logically appear in the outline of a document.

    Hope that helps!!

    1
  • Janelle Schuh•90
    @jschuh23Posted over 2 years ago

    Nice work on your solution Fabricio!! I did notice that the Frontend Mentor report was returning some accessibility and html issues. These should be easy enough to adjust :)

    • For your images, even if they are considered background images they should still have an alt attribute. In this case you would set it as follows: alt="". If you completely omit an alt attribute then a screenreader would read the entire url for the image.

    • The anchor tags that you have listed can probably have the href completely removed to pass the Frontend Mentor test. Meaning don't set them to #. I myself have learned to set blank links to this, but it seems that the FEM test doesn't like those :)

    • I would suggest removing some of the section elements that you have as every section should in turn have a heading element. However, based on your code some sections wouldn't make sense to have a heading. You could probably make do with one section element which includes the entire component.

    Just some suggestions!! Great work!

    Marked as helpful
    0