Skip to content
OverviewSolutions
2
Comments
9
Theo Harris
@Theosaurus-Rex

All comments

  • Theo Harris•140
    @Theosaurus-RexPosted 9 months ago

    Hey Dicksen, sorry it took me so long to getting around to leaving feedback - thanks for your patience!

    Here's a couple of things I found that could help improve this project a bit:

    • Your calculator has form validations which is great, but users are able to enter a negative number of people to split the bill between! You might want to catch this case when validating
    • I can see you've used a <h1> for the logo, but you should use the image provided in the images folder with the template download
    • You can't have an <input> as a child of a <ul> element - you can only have <li> elements inside a <ul>
    • For a form like this, in a production app I would personally be using radio inputs for the percentage amounts, rather than just list items.
    • Instead of using all-caps in your markup on the Reset button, you should type it normally and use CSS to transform it to all uppercase letters using text-transform: uppercase;. This helps screen readers and other assistive technology parse your page better.

    Otherwise, this is a big improvement compared to your previous challenges, nicely done!

    0
  • WillsX9•20
    @WillsX9Submitted 10 months ago

    Hopped on my initial challenge after a few weeks in front-end development. I'm eager to elevate my skills through participation in Frontend Mentor Challenges and contribute to the community. I value and welcome your feedback and suggestions!

    QR Code Challenge Solution With HTML & CSS

    2
    Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey @WillsX9 , congrats on completing your first challenge! 🎉

    There's a few things that can be tweaked here to improve the overall structure of your markup:

    • First of all, the header tag is not appropriate for this use case. Typically this tag is used for things like navigation in a web page. In this case, all of the content is a single group, and all of it can be wrapped in the main tag.
    • When providing alt text for your QR code image here, it's best to describe to the user where that QR code will take them, e.g. "A QR code linking to the Frontend Mentor website"
    • I'd recommend never using <br> in the middle of a line of text. Instead, let the text wrap within its container - this will make your content work better across a range of screen sizes

    Your styling overall looks really good! I think you may have just missed the text colours but otherwise the design looks spot on 🙌

    Marked as helpful
    0
  • Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey, this looks awesome!

    A couple of small suggestions:

    • I'd recommend never actually using all caps in your markup as you've done with some of the headings/subheadings, and instead transforming them using text-transform: uppercase. This helps assistive technologies such as screen readers to have an easier time parsing the page content.
    • I think you might have missed the cream coloured background on the "Transform your brand" and "Stand out to the right audience" sections, but not a big deal 🙂

    Other than that, awesome work!! Really solid underlying structure and the final result looks great! ✨

    Marked as helpful
    1
  • Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey there! Congrats on completing the challenge!

    A couple of small pieces of feedback:

    • I don't think the section tags are particularly necessary here, but if you do want to use them I would be wrapping everything in them rather than just your text - I'd probably have the icon, all of the text and the "Learn More" CTA inside a section each.
    • You should have your heading tags outside of the paragraph tags - these are separate hierarchical elements that should be stacked one after another
    • Use anchor tags instead of button for the "Learn More" CTAs. These are likely to be links to another part of a website, not buttons that a user is using to make a decision. Here is a great article to learn more about this.
    • Your img tags all need to have the alt attribute to be valid. In this case, you can set the icons to have alt="", as they are purely decorative. Here is a handy article that covers how to make your SVGs accessible with alt text if you'd like to learn more.

    Other than that, this overall looks pretty good! Your version seems a little stretched out horizontally compared to the design, so you may want to play with how it looks at a variety of screen sizes as well 🙂

    0
  • Chockplay•60
    @ChockplaySubmitted 10 months ago

    Thanks for view my post. Please fell free to provide feedback, recommentadions, suggestions or code optimizations to improve this project.

    Have fun building!!!

    Qr Code - Frontend Mentor

    1
    Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey @Chockplay, congrats on completing the challenge!

    This looks pretty good - a couple of small suggestions to help improve accessibility and semantics

    • It's reasonable to assume that in a real webpage there would be a <h1>, but in this case the component is isolated, so having a <h2> with no <h1> doesn't make much sense semantically
    • Your QR code image needs alt text, as it's a functional image and not purely decorative - some descriptive text indicating where scanning it will lead the user to is ideal

    Otherwise, great work! ✨

    Marked as helpful
    1
  • Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey @Madeline0421, great work! Looks very close to the design!

    A couple of suggestions for your HTML that will make it more accessible:

    • The image on this card is purely decorative, so you can actually leave the alt text empty to avoid additional noise for screen reader users! Using alt="" is perfectly acceptable here.
    • In your markup, you're using a lot of divs that could be replaced with more meaningful elements
      • The tag holding the date could be changed to a <time> tag - this tag can be provided a datetime attribute, which takes in the publishing date in a machine-readable format. In a real-world scenario, this will help search engines to rank your website content! You can read more about this tag on W3Schools if you'd like to learn more.
      • For text content such as the body text of this card, a <p> tag is preferable, and denotes hierarchy within the markup content
      • The alt text on your author image isn't very descriptive - if you'd like to provide alt text here, try describing the picture like you would to someone who cannot see it, e.g. "a headshot of a man standing against a plain white wall"

    Otherwise, awesome work! I hope this helps!

    Marked as helpful
    1
  • Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey @destinyou, looks really great!

    One small thing that would improve accessibility is changing your <h2> to a <h1>, as it's the top of the informational hierarchy on the page.

    You could also potentially use a <time> tag to encapsulate the publishing date - this isn't always needed, but in a real world scenario where this card is part of a larger blog, it helps search engines to index your page better when returning search results. You can find more information on the specifics over at W3CSchools if you'd like to try it!

    Other than that, great work! I love how smooth the hover animation is 😄

    0
  • Theo Harris•140
    @Theosaurus-RexPosted 10 months ago

    Hey @FatehaRsd, great work on your solution! A couple of small suggestions:

    • The alt text for the QR code image you have currently isn't descriptive, and since the QR code is not purely decorative I would recommend giving it some descriptive alt text to make it more accessible for screen readers and other assistive technologies
    • On my screen, the card itself it a bit off-center, so you may want to check how it looks on a range of different screen sizes (my monitor may have a larger viewport than some, and other folks may have even bigger). Here is a pic of how it looks on my monitor which gives you an idea of what to look out for.

    Other than that, fantastic work! Hope this helps 😄

    Marked as helpful
    0
  • Umutcan Çetin•110
    @NeuralGSubmitted 11 months ago

    -Thats my first responsive website so any feedback would be appreciated

    -What is the best way to decide between buttons and anchor links? I used anchor links here but I'm not sure about that decision.

    Three columns card component using CSS Grid

    2
    Theo Harris•140
    @Theosaurus-RexPosted 11 months ago

    Looks great! Very close to the original design!

    Regarding your question about the difference between anchor tags and buttons, you've made the right choice here by using anchor tags. The key difference is that an anchor tag is used to navigate to a web link, whereas buttons are typically used when a user is making some kind of decision when interacting with a web interface, e.g. a "Submit Form" button, clicking a close button on a popup alert, etc.

    Hope this helps!

    Marked as helpful
    3