Skip to content
OverviewSolutions
27
Comments
35
Antoine C
@mattari97

All comments

  • Antoine C•1,240
    @mattari97

    Hello, I just took a look at your solution and I think you've done a great job. I have two suggestions for improvement:

    Your background doesn't cover the full height of the page beyond 854px in height.

    To fix this issue you can add the following to you body element

    min-height: 100dvh;
display: flex;

    The use of borders on input elements creates a slight "jump" in the layout when they gain focus.

    To fix this issue you have 3 solutions:

    • Play with the property box-sizing
    • Apply a transparent border and only change the color when you gain focus
    • Use outline instead of border

    Have a nice day and happy coding.

  • Antoine C•1,240
    @mattari97

    Hello Duchessa. Good job completing this challenge 🎉🎉🎉.

    I see that you have good basics in front-end and the result is great.

    I will try answer your questions to the best of my abilities and give you some small advises:

    • First of all about the "centering". I think that you were really close to the solution because I can see that you already know about the margin: auto trick with flexbox which worked on the inline axis (horizontal) already. However to make it work on the block axis you need to give a height (or in this case a min-height) to the body element.

    the code would look something like this:

    body {
    /* KEEP Previous styles ... */
    min-height: 100vh;
    display: flex;
    flex-direction: column;
    justify-content: center
}

main {
    /* REMOVE previous styles */
    margin: auto; /* Put the main in the center & push footer at the bottom */
    padding: 1rem;
    width: min(100%, 40rem);
}

    You can remove every over margin: auto in your css and all max-width.

    Note: the property width: min(100%, 40rem); is a shorthand for width: 100%; max-width: 40rem;

    • If you look closely, the border-radius is not "applied" to the image in the .product-card. It happens because the image overflows its container and does not have a border-radius itself. Rather than giving it a border radius you can add the property overflow: hidden to the .product-card which will hide the sharp edges of the image that are "outside" of the section.
    • The image looks distorted when the screen's width is between 380px and 575px. To fix that add the property object-fit: cover to the .product-img class.
    • I would suggest that you add a little transition on the "Add to cart" button for the hover effect. It could be something like this:
    .add-to-cart {
    /* KEEP previous styles */
    transition: 300ms background-color linear;
}

    You can play with the duration and the timing-function as you like until you find something that you like 👍

    <picture>
    <source media="(max-width: 649px)" srcset="./src/assets/mobile/image-header.jpg" />
     <img
       class="header__image"
       src="./src/assets/desktop/image-header.jpg"
       srcset="./src/assets/desktop/image-header.jpg"
       alt="An orange sliced in half" />
</picture>

    That's it. I hope it helps. Have a nice day/night and happy coding 😊

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Hamsalekha. Good job on completing this challenge 🎉🎉🎉.

    I have some feedback for you.

    • I see that you used the float property on many of your elements. If I can give you an advice, it would be to try rethink your layout but using flexbox. Float is hard to use and leads to a lot of side effects. Float is a more modern and simple way to create layouts. Check this link to learn everything about it.

    • I also see that you always wrap your elements with unnecessary containers. You should try to keep you markup as light as possible which will be easier with the flexbox property as well.

    • On my phone the content of your cards is overflowing their containers (the bottom of the text goes out of the card). It happens because you applied a fixed height to your cards. As a general rule you should not set a fixed height on any elements. If you really think you need it; a min-height is ok and will give you elements the ability to grow if the content is too long.

    I would say that the best thing you could do now is try the same challenge with a different mindset. If you have difficulties you can look at this solution from @elaineleung.

    I wish you happy coding and keep going ! Peace 😊

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Clarence, what's up ? Good job completing this challenge 🎉🎉🎉 All basic styles (colors, typography, etc...) seems to be correct so nice work!

    I have 3 advices for you:

    • You should add the property cursor: pointer to all interactive elements, in this case all the links and buttons. This is a small detail that goes a long way for better user experience.

    Now to fix the issues in your report:

    • You should wrap both sections (home & about) in a main tag. You did it already for the header which is great!

    • Same for the <div id="wrongValue" class="notification"></div>. The parent container should be an aside element. It is the landmark you should use for all the elements outside of header, main or footer.

    Have a nice day/night. Peace 😊

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Yaika. Congratulations on completing this challenge 🎉🎉🎉.

    I have some small suggestions to help you improve your solution:

    • I did not try this challenge but it seems that there is a box-shadow applied to the card (your main container) in the original design. It is a really easy fix since you used tailwind. They have some build in box-shadows that you can use. If however you want to create a custom one here is a resource where you will find a lot of different examples. Just create a custom utility like you already did with the @apply directive.

    • There is a small issue with the rounded corners of your image. A simple solution would be to remove the border-radius on the image and add the property overflow: hidden on the main element. The image will then follow the shape of the container.

    There you go, that is nothing really 😊 You did a great job. Happy coding!

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Angel Lam. Great Job completing this challenge 🎉🎉🎉.

    I will try to answer your questions to the best of my abilities.

    • A section wraps a bunch of closely related content. For example on a landing page you could have a hero section , then a features section, etc... In this project using an article is a good choice in my opinion because this is self-contained content but you should replace the sections with some divs. As you know a section should always have a title. Depending on the structure of your page it could be h1 (if you have only one section), h2, h3 etc. If the design does not show a title because the content is self-explanatory for sighted users you should add one anyway for screen-readers and give it a visually-hidden or sr-only class.

    Here is an example:

    .sr-only {
  position: absolute;
  width: 1px;
  height: 1px;
  padding: 0;
  margin: -1px;
  overflow: hidden;
  clip: rect(0, 0, 0, 0);
  white-space: nowrap;
  border: 0;
}

    • aria-label is used on an element that as a meaning that is obvious for sighted users but contains no descriptive text. For exemple an hamburger button to toggle a mobile menu. You know that it is the menu because you can see the icon but imagine that you can't see. Then this is just a button with nothing inside. You could add an aria-label saying "toggle mobile menu" and now it makes more sense. You could also use a span with the sr-only class I showed you which gives the same result.

    • aria-labelledby is used when the descriptive text for an element is placed somewhere else in the code. It is a reference to the id of the element containing the description. For example you could have input and link it to its label.

    • The div element is a tag that you use only to style something. You could use it to apply flex or grid, etc... Anything that is only related to layout and styles. It has no semantic meaning and is useful when nothing else makes sense.

    Hope it helps. Again, good job and happy coding 😊

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hi Alba. Great job completing this challenge 🎉🎉🎉.

    It look really great and minus some small accessibility issues stated in your report that you can easily fix; I have only two small advices to give you.

    I would increase the font-size of the .text paragraphs in your cards and increase the opacity a bit because it it really small right now which makes it hard to read. I am not that old 😁 and even I have some difficulties reading them.

    I would also make sure that the card-image content always stay vertically centered by adding the property align-items: center on the container.

    That's all! Again great job and happy coding 😊 Peace.

  • Antoine C•1,240
    @mattari97

    Hello Aatif Sohel. Good job on completed this challenge 🎉🎉

    It looks good at 375px and above 1280px. However as stated by @Yehan20 you should avoid using fixed sizes.

    The goal is to make the component take the maximum space available but give it the ability to shrink to fit in the screen.

    You have two ways to implement a max-width:

    .my-component {
    max-width: 32rem; /* 1rem = 16px */
    width: 100%;
}

    .my-component {
    width: min(100%, 32rem);
}

    Secondly I think you should have a transition on your buttons on hover and add the :focus selector to make sure the styles also change when the user focus the button with keyboard navigation.

    An implementation could be:

    .my-button {
    color: white;
    background-color: black;
    transition: color 300ms linear, background-color 300ms linear;
}

.my-button:hover,
.my-button:focus {
    color: black;
    background-color: white;
}

    Hope it helps. Happy coding. Peace 😊

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Govindupadhyay good job completing this challenge 🎉.

    Unfortunately I can't review your code because your component is not displayed at your URL.

    To fix this issue you should put all the code relative to your site in the root of your github repository.

    The problem is that right now Netlify can't find the entry point to your app (index.html).

    As a general rule, you should always have an index.html file in the root of your project. Then you can structure the rest however you want if you give the correct paths to the files.

    Hope it helps. Have a good day/night. Peace 😊

  • Antoine C•1,240
    @mattari97

    Hello Ferdinalaxewall. Congratulations on completing this challenge 🎉. Almost everything seems to work fine and it looks really close to the design.

    There is a small issue tho which you might not know about if you don't use Firefox.

    I will always get the same advice because of the caching system of the Firefox.

    To fix this behavior you should have an additional option to your fetch call to prevent the caching of the response.

    I see that you use ajax which i am not familiar with but it should be something like this:

    $.ajax({url: "myurl", success: myCallback, cache: false});

    Anyway. Good job again and happy coding 😀 Peace.

  • Antoine C•1,240
    @mattari97

    Hello Adepoju Adeyemi Joshua. Congratulations on completing this challenge 🎉. It is really well made so good job overall!

    I have some small suggestions for you regarding minor problems:

    • You should probably try to rethink you layout using grid. It is the perfect opportunity to use this awesome CSS property. The design is a lot of very similar cards in a repetitive order which is a perfect match for grid. This awesome article will help you get started.

    • You forgot to update some styles on dark mode like the icons & color of the font in your input-div

    • I would use a <header></header> element instead of a <div class="header"></div> for your header which would add more semantic value to you HTML and fix a lot of warnings in the report of your solution.

    • I would add a link on the Where in the world? text which redirects to the homepage. This is a feature everyone expects on a website nowadays and really quick to implement.

    • I see that you used h4 tags for all your titles. I guess you did it for a styling purpose (in this case the font-size) but this is not a good practice. heading tags should have a logical order in the page. First a single h1 then one/multiple h2 etc... Then you would style them using CSS according to your needs. This is especially important for accessibility.

    • Lastly regarding the images of the cards on the home page; I would add a min-height to make them look closer to the design and also add the property object-fit: cover to prevent any distortion.

    Fell free to ask any question if you have some trouble fixing some of these issues.

    Again good job with your solution and happy coding.

    Peace 😊

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Katherine. Congratulations on completing this challenge.

    Regarding your question about the filters. I think local-storage is a really good solution if you need to keep the values when the user closes/refreshes the browser but if you just need to keep the "state" when your switch between components/pages you might wanna use a state management solution. I have three of them in mind so take a look and see if it fits you requirements:

    1- The Context API. It is the built in solution in React but can be a bit hard to setup and quite verbose. However it means no additional dependencies which is always great 😉

    2- Jotai Very simple and flexible state management library for React.

    3- Zustand. This is my favorite. Really awesome state management library.

    Each of these solution allow you to "share" some state between components/pages.

    Hope it helps. Happy coding. Peace 😀

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Mona Elshikh. Congratulations on completing the challenge. I just made the same one and i think it was really fun 😊.

    I have some small suggestions for you:

    • I think that you forgot to hide the hamburger button on desktop. Because right now i can see it and click on it even on larger screens. You might want to add a display: none at your breakpoint of choice.

    • Also the "Shorten it" button has two small problems:

    1- The text is breaking on two lines at ~1200px which can be easily fixed with a white-space: nowrap on the button.

    2- To make sure it always stays at the same height as the input you could add align-self: stretch

    Hope it helps and happy coding 😀

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Victor Okeke. Good job on completing the challenge. This is a good work. It looks really close to the design which is great!

    I have some small suggestions for you:

    • You might want to make sure the the content is always centered on the screen by adding some properties to the body element:
    body{
    font-size: 13px;
    font-family: 'Barlow Semi Condensed', sans-serif;
    background: hsl(210, 46%, 95%);

    display: flex;
    min-height: 100vh;
    justify-content: center;
    align-items: center;
}
    • I see that you used min-width for your container on desktop but instead i would use this:
    .container{
      min-width:1200px; -- Remove this

      max-width: 92rem;
      width: 100%;
}

    You could also use the CSS function width: min(100%, 92rem) which gives the save result.

    1rem = 16px feel free to experiment with it to keep it close to the design. Because right now it looks really stretched on large screens.

    Again good job with the solution and happy coding 😀

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Ghulam Mohyuldin. Congratulations on finishing your first project on the site. It is a good work!

    I have some small suggestions for you to make it a bit closer to the original design:

    • I see that you user some rather large margins on your main component. I guess you did it to center it on your screen. But there are more consistent solution for this using either flexbox or grid. In fact on my screen it is not centered with your current solution. Here is a little snippet of code that would fix this issue:
    body{
    background-color: rgb(204, 242, 248);
    font-family: 'Poppins', sans-serif;

    min-height: 100vh;
    display: flex;
    flex-direction: column;
    justify-content: center;
    align-items: center;
}
    • Also instead of using % to size your card i would give it a max-width like so:
    .imgandcont{
    width: 16%; -- Remove this
    margin: 100px auto 100px auto; -- Remove this

    max-width: 18rem; -- Add this
    margin-bottom: 2rem; -- Add this
    
    border-radius: 12px;
    text-align: center;
    background-color: aliceblue;
}
    • Lastly instead of giving a property width: 89% on your img to prevent it from overflowing its container; i would add a rule in the top of my css file like so :
    img {
    display: block; 
    max-width: 100%;
}

    This last rule would be part of the CSS Reset/Normalize which you should add to all your projects. Here is an example that you could adapt to your projects.

    Again good job with your solution and happy coding 😀

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hello Kounik. Congratulations on completing this challenge. It is well done.

    However i have some suggestions for you:

    • Regarding your question I think the problem is that on the design each card has it's own shadow rather than the all section of card. You might wanna try this and see if it matches with the design files. It will also remove the "hovering effect" & sharp edges it gives to you card section in the current solution.

    • I would also add a bit more spacing between each card title and the review text below.

    • If you open your browser developer tools (with F12 for chrome and firefox) and look at the mobile version of your site you will see that you can't scroll through the page. This is because you set a height: 100% on your body. To fix this you should use the min-height property instead. I would also add some vertical padding so the first and the last card don't stick to the window borders.

    body {
  height: 100vh; -- Remove this

  min-height: 100vh   -- Add this instead
  padding-block: 1rem

  width: 100%;
  background-color: var(--lgrayish-blue);
  font-family: var(--barlow);
  font-size: var(--fond-md);
  display: flex;
  justify-content: center;
  align-items: center;
}

    Again good job and happy coding. Peace 😀

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hi Elaine. Almost pixel perfect ! Impressive.

    I will look at your theme switcher tomorrow. I was going for a similar solution with 3 clickable areas but I went back to a single clickable area because I thought that on mobile I often was clicking on the wrong one because the switch on the design is quite small.

    Awesome work! Peace :)

    Marked as helpful
  • Antoine C•1,240
    @mattari97

    Hi Oduh Precious. Congratulations on completing this challenge.

    To change the color of an svg using classes, you need to remove the hardcoded colors in the .svg file and replace it with "currentColor". This way you can manage the color of the svg with the CSS color property like it was text.

    Hope this is helpfull. Have a good day/night. Peace ✌️