Skip to content
OverviewSolutions
4
Comments
4
Nick
@Fitsos17

All comments

  • Nick•110
    @Fitsos17Posted over 2 years ago

    Hello mate, your solutions is very close and I like it. What you can do is to fix those accessibility issues.

    • Wrap all your content inside a main tag
    • Use an h1 heading instead of h5 That should solve the issues. Also, note that the text on your heading wraps at front-end and not at by. Reduce the width of the heading and you'll be good to go. Also, consider making the image's width smaller and the overall height smaller. This should fix your solution. Thank you and keep up the good work 👍
    Marked as helpful
    1
  • Nick•110
    @Fitsos17Posted over 2 years ago

    Hi man. Nice solution. I just want to help you fix those accessibility and HTML issues.

    • Don't forget to add an alt in all of your img tags. It helps for screen readers.
    • The error All page content should be contained by landmarks can be solved by wrapping the whole app in a <main></main> tag.
    • Page should contain a level-one heading. This says that you should add an <h1></h1> tag. Try replacing every span and p tags with headings from h1 to h6.

    These should fix the issues. Very nice solution. Keep it up, Nick.

    0
  • Nick•110
    @Fitsos17Posted over 2 years ago

    Hello. Great solution, though you can fix some parts.

    First of all, in JS change the following:

    • On fetch, add this: fetch(URL, {cache: 'no-store'}). By adding this, you'll be able to fetch the advice in firefox browsers. I had this problem too.
    • It is bad practice to use .innerHTML. Use .textContent instead. Check this link for more information.

    In your css: Do what @malboyoo told you to do and

    • Import the 800 weight font. You have imported the 400 weight font.
    • On your body, use min-height: 100vh;. By doing this, I think that everything will be centered.
    • Consider making the font size of your quote bigger.
    • There is no need for border radius in quote-btn. Add it directly on btn-container. This shouldn't change the border-radius of the image and it'll change the container's border-radius, which is what we want.

    I hope I helped with this. Very nice work though 👍 Keep it up, Nick.

    Marked as helpful
    0
  • Johnsonworld•60
    @JohnsonworldSubmitted over 2 years ago

    This is my second project using Javascript. I was able to get the functionality I wanted for the most part but my JS is clumsy. Mainly I think I could have encapsulated it better instead of having so many snippets of code. Any feedback about that would be great.

    Thanks!

    Tip Calculator Using Vanilla Javascript

    2
    Nick•110
    @Fitsos17Posted over 2 years ago

    Hey man. Great job with your solution👍! One thing that I noticed searching your code is in your header tag you said <h1>SPLI<br />TTER</h1>. I think that there is no need for that. Check images folder and you'll find an svg file that is named logo. You can use this directly to your app via an img element. It contains everything you wrote. Just style that img tag and you'll be ready to go!

    Marked as helpful
    0