Skip to content
OverviewSolutions
11
Comments
5
Halí V.
@halivert

All comments

  • Halí V.•230
    @halivert

    Hey @Notyan21 this is a great solution to the challenge, I just want to make some suggestions here:

    • Using of dynamic viewport units
    • You can add a color to box-shadow after the last dimension, to make it look transparent, e.g.:
    box-shadow: 2px 2px 14px -10px rgba(0, 0, 0, 0.2);

    Also I want to congratulate you for the following points:

    • Great use of CSS properties
    • Nice class naming (good BEM practices)

    Keep up with the good job 🎉

  • Halí V.•230
    @halivert

    You could use the toFixed method result.toFixed(2)

  • Halí V.•230
    @halivert

    There are some issues with the error icon in the live site, the fixed route for the icon is: src="images/icon-error.svg"

    Good job 👍🏽

  • Halí V.•230
    @halivert

    For your three accessibility issues:

    1, 3. Document should have one main landmark

    You could wrap the three cards inside a <main>

    1. Page should contain a level-one heading

    Sections could have an h1 level heading

    Also the way you write your styles is great for modularity.

    Good job 👍🏽

  • Halí V.•230
    @halivert

    I like how you use first-child and last-child pseudo-classes, that's a nice approach.

    If you want to use semantic HTML5, I recommend read this article https://developer.mozilla.org/en-US/docs/Web/HTML/Element

    Also I recommend to use Vue.js components, instead repeating tree times the same (or almost the same) code.

    Good job 👍🏽