Skip to content
OverviewSolutions
5
Comments
11
P
Nikolas Escobal
@nikoescobal

All comments

  • P
    Nikolas Escobal•230
    @nikoescobalPosted over 2 years ago

    Hello Omar! 👋

    Congratulations on finishing your challenge! 🎉 Amazing job! Looks really close to the original!

    Here's some feedback/suggestions on this solution:

    • I recommend learning Sass as it helps keep code clean and easy to understand since there's the added ability to be able to nest classes. This enables you to mimic the structure of the HTML in your CSS.
    • When clicking the add button, it seems the cart on the top right gets updated, but not the text with the price itself. You should make this dynamic rather than static.
    • I'd recommend practicing building layouts without relying on absolute positioning unless necessary -- you can achieve much of the same results with just padding and margins. This would be especially helpful when it comes to adjusting the responsiveness of your website across any screen size
    • Check the report to fix accessibility/HTML issues - you currently have 11 accessibility issues and 3 HTML issues listed

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted over 2 years ago

    Hello Julien! 👋

    Congratulations on finishing your challenge! 🎉 Unfortunately, you haven't properly gotten the site deployed, as you have a 404 error.

    Here's some feedback/suggestions on approaching this solution:

    • You should build mobile-first, meaning all your CSS classes should apply to the mobile viewport first, then add media queries to manage the other screen sizes
    • Add padding to the text so it doesn't touch the sides of the card.
    • Check the report to fix accessibility/HTML issues - you currently have 10 accessibility issues and 2 HTML issues listed

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted over 2 years ago

    Hello Danny! 👋

    Congratulations on finishing your challenge! 🎉

    One lesson that really stuck with me, which I got from Kevin Powell, is that the web is responsive by default, meaning that any time you build a project, as long as you don't mess with negative margins or absolute positioning, your content is by default, responsive. That said, it makes so much more sense to go with a mobile-first approach, then meaning all your CSS classes should apply to the mobile viewport first, after which you then add media queries to manage the other screen sizes. Doing it this way also makes it easier to scale and adjust in the future, even if you will add changes to your layout.

    As for using flexbox vs grid, it's best to practice both. For anything that has to do with tables, you should use the grid approach, whereas flex can be used to manage layouts that are simpler. Given that you ran into the difficulty of trying to make the columns the same size, the only solution is to use grid-template-columns with the fractional (fr) value, which guarantees they will always be the same size. An example of this looks like this grid-template-columns: repeat(auto-fit, minmax(min(320px, 100%), 1fr));; Try giving it a shot -- the best way to learn is by practicing and applying new things little by little.

    Here's some other feedback on this solution:

    • It seems like the filtered image is a bit too dark. You could either adjust the opacity or instead use background-blend-mode or mix-blend-mode with the property set to multiply.
    • For your text below the numbers, you're not using the correct font, font size, and font weight

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted over 2 years ago

    Hello there! 👋

    Congratulations on finishing your challenge! 🎉

    Here's some feedback on this solution:

    • You should build mobile-first, meaning all your CSS classes should apply to the mobile viewport first, then add media queries to manage the other screen sizes
    • It seems like you didn't resize the fonts for mobile view
    • Add padding to the text so it doesn't touch the sides of the card.
    • Check the report to fix accessibility issues - you currently have 1 listed

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    1
  • P
    Nikolas Escobal•230
    @nikoescobalPosted over 2 years ago

    Hello there! 👋

    Congratulations on finishing your challenge! 🎉 Feel free to check out my project here, and the source code here

    Here's some other feedback on this solution:

    • You can add min-height to the tables for each breakpoint. It looks quite small on desktop
    • Your buttons lack padding, as they're considerably smaller compared to the original design
    • You could do with increasing the grid gap for desktop
    • It seems like you didn't apply the correct letter spacing, font colors, and padding for the 1200px breakpoint
    • Check the report to fix accessibility issues - you currently have 1 listed

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted almost 3 years ago

    Hello there! 👋

    Congratulations on finishing your challenge! 🎉

    Here's some other feedback on this solution:

    • Keep in mind that the web is responsive by default
    • This is why it's suggested to always start with mobile-first, then use media queries to cover the other screens
    • Check the report to fix html issues - you currently have 2 listed

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    1
  • P
    Nikolas Escobal•230
    @nikoescobalPosted almost 3 years ago

    Hello there! 👋

    Congratulations on finishing your challenge! 🎉

    To answer your question, there are multiple ways to approach this. One way you could use is to create black circles that have the same color as the outside, then adjust the positioning using margin to create that "cut effect."

    Here's some other feedback on this solution:

    • you forgot the social media icons below
    • add letter spacing for your title and text below
    • the spacing in between your cards is too much - you should adjust the padding/gap
    • check the report to fix accessibility issues - you currently have 3 listed

    I hope this is helpful and all the best with your coding journey!

    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted almost 3 years ago

    Hello there! 👋

    Congratulations on finishing your challenge! 🎉

    I think using negative margins is completely fine, although it should only be used when necessary. The alternative is to use absolute positioning -- keep in mind that if you do use absolute positioning, the parent container must have a position relative applied to it.

    Here's some other feedback on this solution:

    • you forgot to add the quotation marks
    • You can add width to your content + text-align-center to make it look like the original design
    • check the report to fix accessibility issues - you currently have 5 listed

    I hope this is helpful and all the best with your coding journey!

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted almost 3 years ago

    Hello there! 👋

    Congratulations on finishing your challenge! 🎉

    I have some feedback on this solution:

    • I suggest you build mobile-first moving forward, as your code currently breaks upon resizing to 375px
    • The web is responsive by default, so just add media queries to cover bigger screens
    • check the HTML report to fix accessibility issues
    • Your button content is currently overflowing because the font size is too big.
    • You can also set a max-width for your button so it doesn't stretch outside the container

    I hope this is helpful and best of luck with your coding journey!

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted almost 3 years ago

    Hey! Good effort, but it seems like your project isn't compiling. The live link is broken. Do update it so that the community can provide feedback on your solution! Also, other than GitHub Pages, I suggest try exploring Netlify -- it's fairly straightforward to deploy projects from there. All you need to do is import the file then deploy.

    Marked as helpful
    0
  • P
    Nikolas Escobal•230
    @nikoescobalPosted almost 3 years ago

    Hey there! Great job with this first try. This video was helpful for me - https://www.youtube.com/watch?v=uLvhAJfx3T0&t=3s

    Also, you seem to have too much letter-spacing for some of your text. It also looks like you haven't made this fully responsive. I'd recommend starting with mobile-first. You'll have to spend some time to grasp the fundamentals of flex-box and grid to be able, but the good thing is that this will definitely get better with more practice. I recommend checking out Kevin Powell's videos - he has tons of content on responsive layouts, and many of them are pretty short! All the best with improving!

    0