Hello! It was a funny challenge. I had problems using the <picture> tag, didn't use it, but still managed to get a result I love. All kinds of problems arised when I tried to center <main> with flexbox. Getting it pixel perfect as the guide image wasn't my point. The result? I left it as it was ahah :) I used one media query just to resize the picture height and I'm happy with it. What do you think? Any suggestions? Any ideas of things I could have done easier than I did?
Antoine C
@mattari97All comments
- @duchessa01Submitted about 2 years ago@mattari97Posted about 2 years ago
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 allmax-width
.Note: the property
width: min(100%, 40rem);
is a shorthand forwidth: 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 propertyoverflow: 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 π
- And finally here is an example of a picture element from one of my solutions :
<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 helpful1 - 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
- @LekhaKumarSubmitted about 2 years ago
I tried to create a responsive design for desktop, tablet and mobile. I am open to suggestions and tips for improvement. Thank you.
@mattari97Posted about 2 years agoHello 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 helpful1 -
- @clarencejuluSubmitted about 2 years ago
I decided to add a backend for this project using JSONServer and Heroku, to store the amount of items left, progress bar info etc. I also implemented localstorage to store whether the project has been bookmarked or not. Please review this project and let me know your honest opinions. Thank you in advance!
@mattari97Posted about 2 years agoHello 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 anaside
element. It is the landmark you should use for all the elements outside ofheader
,main
orfooter
.
Have a nice day/night. Peace π
Marked as helpful1 - You should add the property
- @YaikaRaceSubmitted about 2 years ago
What I found difficult was to make the small triangle that has the text bubble shape, from my device I can see a small gray border and I did not know how to fix it, any suggestion and opinion that helps me to improve is welcome.
@mattari97Posted about 2 years agoHello 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 helpful1 -
- @Azn4n6elSubmitted about 2 years ago
I think the structure was the most difficult thing for me in this project. I am unsure of my accessibility practices, when using section should I always include a label? differences between aria-labelledby and aria-label. Is div really necessary when you have all these landmarks?
@mattari97Posted about 2 years agoHello 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 helpful1 - @albscrSubmitted about 2 years ago
Hey there! Here is a new challenge, I hope you like it. Looking forward to your recommendations. Thanks!
@mattari97Posted about 2 years agoHi 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 propertyalign-items: center
on the container.That's all! Again great job and happy coding π Peace.
0 - @aatifsohelSubmitted about 2 years ago
Hey everyoneπ
What do you think of my code?
How can I improve my coding skills?
If you have any suggestions, feedback is appreciated!
@mattari97Posted about 2 years agoHello 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 helpful2 - @IntellectGoviSubmitted about 2 years ago
*I am just facing problem to create a responsive website. *And I think that there are some lines that can be reduced by some better tags and codes.π
@mattari97Posted about 2 years agoHello 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 π
0 - @ferdinalaxewallSubmitted about 2 years ago@mattari97Posted about 2 years ago
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.
0 - @The-indigoSubmitted about 2 years ago
Had a hard time figuring out using flex alone for responsiveness of the cards while still maintaining their width on different screen sizes (min-width and flex wrap) and resorted to using media queries which felt a bit hacky.
@mattari97Posted about 2 years agoHello 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 helpful1 -
- @rendongzhaSubmitted about 2 years ago
Hi everyone, I use localStorage to store and get the search and filter conditions so that they are persisted after I click through to the detail page and then come back. Is there a way to persist the input value while using useRef hook? In the SearchFilter component, I was using useRef to refer to the search bar value, but I found it difficult to initialize the reference value, so I changed to useState hook, which works well. Thanks in advance for any suggestions!
@mattari97Posted about 2 years agoHello 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 helpful1 - @MonaElshikhSubmitted about 2 years ago
Hi Everyone, I am happy to share my work with you, it was a nice challange , there was one challancing thing which is parsing the shorten links from localstorage, it did not work, it gaves a string even after JSON.Parse(), so after long search i found that it must be parsed twice.
i will appreciate your comments and modifications
Thanks a lot.
@mattari97Posted about 2 years agoHello 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 helpful1 -