Skip to content
OverviewSolutions
16
Comments
14
Arthur Roberts
@arfarobs

All comments

  • Arthur Roberts•410
    @arfarobs

    Hey. Great work so far Casparas.

    I suggest wrapping your <div class="qr-code"> with a <main> element. This will improve accessibility.

    When using headings, you should always start with a <h1> and work your way down from there. If your document doesn't have a <h1> heading, then it shouldn't have a <h2> heading. I suggest replacing the <h2> with an <h1> element.

    Hope it helps.

  • Arthur Roberts•410
    @arfarobs

    P.S I've just noticed the planet image size, HTML, and accessibility issues. I will fix them over the next few days.

  • Arthur Roberts•410
    @arfarobs

    Hey Angelo. I'm currently working on this project myself. I like the hover effects you put on the images.

    I've got a suggestion to neaten up your code with the grid images. It's not good to have three <img> tags for the same picture. This has something to do with accessibility and SEO. I spent hours looking for a solution to this. I will share it with you.

    Instead of this:

    <div class="mobile-grid1"> <img src="assets/mobile/image-grid-1@2x.jpg" alt="grid1"/> </div> <div class="tablet-grid1"> <img src="assets/tablet/image-grid-1@2x.jpg" alt="tablet-grid1"/> </div> <div class="desktop-grid1"> <img src="assets/desktop/image-grid-1@2x.jpg" alt="desktop-grid1"/> </div>

    I'd suggest using this:

    <picture class="img-grid-1"> <source media="(min-width: 1440px)" srcset="./resources/assets/desktop/image-grid-1.jpg"> <source media="(min-width: 768px)" srcset="./resources/assets/tablet/image-grid-1.jpg"> <source media="(max-width: 375px)" srcset="./resources/assets/mobile/image-grid-1.jpg"> <img src="./resources/assets/mobile/image-grid-1.jpg" alt="The galleries main room."> </picture>

    This will avoid confusing search engines. Also, the pictures will automatically change to the correct file without adding them to your media queries in CSS. If I'm correct, I think this will also help loading speeds because the browser will only load the image that it needs, but i could be wrong about that.

    Hope it helps.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Hey Vedat. It's looking good. I have a few suggestions for you.

    Accessibility:

    1. All of you images should have an alt attribute with a description of what is in the image as a value. For decorative images like the icons, Id suggest giving them an empty alt attribute. https://developer.mozilla.org/en-US/docs/Web/API/HTMLImageElement/alt

    2. As Shashree mentioned above, your content should be wrapped in a <main> element. I also noticed that you wrapped the heading in a <p> element. I suggest that you change that to an <h1> element. All webpages should have an <h1> element. The link below explains the best practices when using headings. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

    Flexbox:

    1. You said you had issues with flexbox. I will leave two links below. These links are games in which you use flexbox as the controls. They are great for practicing flexbox. They're also quite fun and explain a lot to you. https://flexboxfroggy.com/ http://www.flexboxdefense.com/

    I hope this helps you. I have also completed this challenge. I used flexbox in my solution. Feel free to refer to it.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Hey. If i was designing this just from the picture. I would use the text as a method to scale a close size. In the downloaded files for the project there should be one that tells you what styles to use. This will tell you the font sizes, colors, etc.

    In the design picture you can see that the heading is two lines. After the word "front-end" you will see that the line breaks. By looking at it this way you can make a better estimation for the components width.

    In a project like this i think its best to try and style things in relation to the information that you are given in the style guide.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Hey, Angelo. Great job on learning the basics of CSS Grid. I have a few suggestions for you, mate.

    1. If I was to do this project, I would put the line "join our community" as an <h1> heading. This will avoid the accessibility error in the report.

    2. To fix the issue with the media query, you can change line 126 from this: @media (max-width: 1600px) and (min-width: 800px){ to this: @media (min-width: 800px){ By doing this you are saying that you want your media query to work on all screen sizes with a width of 800px+.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    This might be useful for you. It's a game that will go through the basics of CSS grid.

    https://cssgridgarden.com/

  • Arthur Roberts•410
    @arfarobs

    下午好呀！看起来很棒。I have a few suggestions for you.

    1. I suggest adding some attributes to the <img> icons. <img src="./images/icon-sedans.svg" alt="" aria-hidden="true"> If an image is decorative, you should add an empty alt attribute. Then, add an aria-hidden="true". This will hide the element from accessibility tools.

    2. I suggest wrapping your content in a <main> element. Landmark elements are important for accessibility.

    3. I noticed that nearly everything is wrapped withing <div>s. It would be better to use some other elements.

      • For headings use: <h1> <h2> <h3> <h4> <h5> <h6> in that order.
      • For paragraphs use: <p>
      • For buttons use: <button>

    I hope this helps. 因为我注意到你来自台湾，所以我觉得你会说中文。下面有两个网站， 第一个是真的很有用，有中文和英文。

    • https://developer.mozilla.org/zh-CN/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_aria-hidden_attribute
    • https://www.w3.org/TR/wai-aria-practices/examples/landmarks/HTML5.html
    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Hey Harel. I have a few suggestions to help you improve your solutions.

    1. The image doesn't display properly. I noticed that there is an error in the <img> src attribute. Your code: <img src="/images/image-qr-code.png" class="card__img" alt=""> Corrected version: <img src="./images/image-qr-code.png" class="card__img" alt=""> The first / needs a full stop in-front of it.

    2. To avoid the errors in the report, there are two things that you can do.

    • Wrap your HTML in a <main> element.
    • Instead of using an <h3> element use an <h1> element. Headings are supposed to follow a kind of hierarchy system. If you only have one heading element, it should be an <h1>. Every page should only have one <h1> element. Then after that use <h2> then <h3> <h4> etc. depending on the importance of the heading.
    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Hey Angelo. I'm not the best with grid, so i can't answer that question. However, I have a suggestion when it comes to accessibility.

    I notice that you have wrapped your HTML in a <div class="main">. To avoid getting the errors in your report, it would be better to wrap it in a <main> element.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Also, I always seem to have problems with my typography. Any feedback on how to improve that would be great.

  • Arthur Roberts•410
    @arfarobs

    Hey Karim. I just completed this project. I used Figma as apposed to sketch.

    To try and answer your question. I would assume that character stands for letter-spacing in this case it doesn't have a value. Line would stand for line-height and in your pictures case it would have a value of 28px. Paragraph stands for paragraph-spacing in your pictures case has a value of 1.

    I hope I've read and understood your question properly.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Hey man. Good job on completing the challenge. I got two bits of feedback for you.

    1 - I made this same mistake when I did this challenge. You forgot to add shadow to the box.

    2 - If you want to get rid of the accessibility errors in your report. Wrap your content in a <main> tag and add an <h1> tag to it.

    Hope it helps.

    Marked as helpful
  • Arthur Roberts•410
    @arfarobs

    Accessibility WARNING Page should contain a level-one heading

    Context:

    <html lang="en"> Learn more

    Also, if anyone could explain why I have this message in my report that would be great.