Skip to content
OverviewSolutions
7
Comments
9
Michael
@mksoofian

All comments

  • Michael•240
    @mksoofian

    Hi Lucas,

    I totally get what you mean. It can be difficult for me to come up with my own layout for the in-between screen sizes... you did an awesome job with the alternative layout! tips hat

    My constructive criticism would be for you to adjust the cart qty to display accurately in the pop-up above the icon. Otherwise, I think you provided a great solution.

    -Mike

  • Michael•240
    @mksoofian

    Hi @visualdenniss,

    Your project looks great! The positioning and animations are near perfect.

    Here are some constructive comments of what I found did not exactly meet the brief:

    • The light box should only be showing when the large image is clicked. Your solution activates the light box when clicking a thumbnail when that should really be updating the large image.
    • The Add to Cart button feature only populates a "1" in the qty bubble of the icon when it should reflect the number in the cart
    • The qty in cart seems to be hard coded to "3" when it would ideally reflect the qty of the line item (connected to the qty bubble and the add to cart button)
    • In addition to my previous note, the trash/delete icon should decrement the cart qty and register as "empty" when it reaches zero or empty

    I think implementing the above improvements would really take your project to the next level. Good luck!

    Marked as helpful
  • Michael•240
    @mksoofian

    Hi @ishitaraina1807!

    Congrats on getting started here. Here are some of my suggestions that I hope you find helpful:

    • Delete everything in the <body> of your HTML except the <div class="body">. All that content is not needed for your project.

    • Take a look at your file now, it should look a lot better and more simplified! =]

    • A more appropriate way to center your card on the page would be to add the follow CSS to your <body> tag display: flex; justify-content: center; align-items: center and also height: 100vh (this will adjust your body element height to match that of your browser window)

    • Remove all the margin selectors as well as your width selector. Instead apply a max-width: 17em and your height will auto adjust as needed.

    It might not perfectly match the original design but you can tweak if needed. I hope this process of adjusting your code was helpful. If so please upvote my comment :)

    Good luck on your journey!

    Marked as helpful
  • Michael•240
    @mksoofian

    Hi @nikitaaksjonov,

    This is an amazing first attempt! Very impressive.

    The only main design difference is the layout of your items within you list <li> elements. I think it would be improved by using:

    1. Rather than <span> elements, <p> or <div> would be more suitable here.
    2. There are various ways to do space these but I will give one example here: Group the item-picture and item in one div and give it a class something like "category" and then group the other two value elements in a div and give it a class of "cat-results". You can then add to .summary-container ul li {justify-content: space-between} which move them to the ends of the "reaction container. Then I would adjust the spacing accordingly.

    I hope this helps! Best of luck!

  • Michael•240
    @mksoofian

    Hey Abdel,

    Nice first attempt here. I'm no pro but here is some feedback that I hope is helpful to improving your code:

    -You can remove the <div class="attribution">, it is only meant as a placeholder in the starting code. Removing it will help you execute your project more accurately as well.

    -This is not a responsive project, or at least not required. Therefore, you can remove the media query. Additionally, I tend to format it as @media (max-width: XXpx). When the screen size is less than the max-width, it will execute the code within your media query.

    -Remove from Body:

    • margin: 26px
    • width: 90% After I did this, your design actual centered better on the page. I would actually recommend setting the margin to "0" as the Browser automatically is putting on some margin here and we should reset that unless you intentionally want it there.

    -It is good practice to fill out the alt=" " in your <img>. This is what they call "accessibility" in coding. People who are visually impaired, the screen reader will read them what the alt=" " is since they can't see there is a QR code there. So, in this case you would write something simple like, alt="QR code".

    -You can remove the font weight on the main <p> text as it is not doing anything really. It is superfluous as your font is already at that weight.

    There are a few approaches to getting the <p> to split into three rows but I will spare you unless you want help. Let me know.

    TIPS: I checked your site by using the chrome dev tools. I would suggest you familiarize yourself with how to use it. It makes really easy to play with without affecting your actual code.

    Also, check out Kevin Powell on youtube. He is really fantastic to watch on youtube and explains things very well. I have and continue to learn a lot from his content.

    Wishing you the best of luck on your coding journey! Mike

    Marked as helpful
  • Michael•240
    @mksoofian

    Hey Thanh,

    I really love your accuracy with your aspect ratio. I did not take the time to look through your code as I am not familiar with Vite but I wanted to note that Solution provided is not very responsive. The bars become squished and the component becomes cut off from view as the viewport shrinks. I would recommend improving this so that your component can be viewed on devices of almost any size.

    Enjoy!

    Mike

  • Michael•240
    @mksoofian

    Update:

    I could not get the border color to change on focus because by default, the browser was highlighting the input a different color. My solution was setting "outline:none", only then would my border styling be revealed.

    I wasn't able to get my results to calculate as I input numbers in the text field. It would only update after clicking away from the input. This was especially if using mobile as users wouldn't know to tap away from the input in order for the result to be rendered. My solution was to run my updateAll() function in the event listener for input.

    I hope this helps others with this challenge.

    -Mike

  • Michael•240
    @mksoofian

    Hi Tiago,

     I really appreciate your submission because your code helped me overcome the challenges I left unsolved with my code. I also only used vanilla JS and CSS!

    1. I could not get the border color to change on focus because by default, the browser was highlighting the input a different color. My solution from your code was setting "outline:none".

    2. I wasn't able to get my results to calculate as I input numbers in the text field. It would only update after clicking away from the input. My solution based on your code was to run my updateAll() function in every input event listener.

      This actually revealed to me bugs in your code. Your app will only work if the bill, tip and number of people are provided in the vertical order of the layout. However, app crashed if number of people is entered first, it also will not let me change the tip% after my initial input/button selection. I would suggest to update the code to update results after every input as I learned in my second point above as we can probably expect users to manipulate and edit fields for any number of reasons.

    I hope this feedback helps!

    -Mike -Mike

    Marked as helpful
  • Michael•240
    @mksoofian

    Hi,

    Nicely done!

    A tip to help this more easily viewable would be to set the .container width to about 500px.

    Since min-content is currently being used, your container is larger than the viewport height which causes the user to need to scroll. By decreasing the width to a more standard size, it will be more readily viewable on all devices.

    Kindly reminding that smaller devices have a width of 375px while standard screen sizes are at 1440px.

    Good luck on your practicing! -Mike

    Marked as helpful