Not the best code I've written , both react and scss, you can checkout my github readme for more explanation . . . anyways there are some issues I would gladly feel happy to be helped with.. like resetting the state of my cart checkout when delete btn is clicked , or my css as well . . . anyways cheers 🥂🥂
Great Challenge @kabir-afk
At first sight, contribution i can make is that when one click on the button after invoking addToCart , It will be a good behavior to set back numberOfItems to 0 like the following:
<button
className="addToCart"
onClick={() => {
props.addToCart({ numberOfItems })
setNumberOfItems(0)
}}
>
Hi @ttsoares nice challenge
First impression i wanted to put emphasis on is the structure of the todos array
{ id: "item-1", content: "Complete entire javaScript course ", completed: false, /*If the checkbox is "checked", the "completed" property will have "true" as value/ And your checkbox "unchecked" will result this property becoming "false" */ },
{ id: "item-1", content: "Complete entire javaScript course ", active: true, /*If the checkbox is "checked", the "active" property will have "false" as value/ And your checkbox "unchecked" will result as "true" for this property */ },
And your toggleCompleted function should take the id as argument due its uniqueness
function toggleCompleted(id ) {/*instead of "todo"*/
const result = todos.map((elm) => {
if (elm.id === id) {
return { ...elm, completed: !elm.completed };
} else return elm;
});
setTodos(result);
setBackTodos(result);
}
And as a result you'll also call toggleCompleted or remove or toggleActive functions with the id of your task like so :
<div onClick={() => toggleCompleted(todo.id)}></div>
At first sight, these are some point that i can highlight on your code; And i hope this will help you improve your solution
Everything is working except sometimes the screen refreshes really fast. I can tell the code is working but I don't know why the error state or the success screen flashes so quickly. Does anyone have any solution or input for this?
Nice challenge @arogersrenee .
As an answer to your question , the fast refresh is caused by the action of submitting the form .
And you can prevent it like this
form.addEventListener("submit", (e) => {
e.preventDefault();
//Instead of e.preventDefault
validateEmail();
})
It looks good on mobile screens
But i think it will be also nice if you fix the responsiveness on larger screens
Besides this i think it's all good
Great challenge @ahmetmetinarslan One suggestion that i could make is to add margin to headers and images so you can make it more close to the design
Besides that i think it's all good in my opinion
Great challenge @AdeMarq, And i think it's all good but the only thing that i may suggest is to add transition to your buttons to smoothly change properties when passing from "normal" to "hover" state : button{transition: all 0.3s ease}
You can also add the cursor property to all your buttons in one place instead of putting cursor:pointer to each hover state of each button you could just do button{cursor:pointer} and it will be applied to all your button element
There is some padding that you can add especially to the top of the container when adding breakpoint :
@media (max-width: 930px){
.container{
padding-top:100px;/*You can also adjust the value to your need*/
}
That's in my opinion some improvement that can be made . Hope that you will find it helpful
I read all the comments below thank you
Hi @lamriouiaya20 great challenge, But it feel like you use absolute units a little bit too much , instead of using absolute units like "pixels" for sizing your grid you could for example use relative units like "fr" for allowing your grid to be more flexible
.contenu {
max-width:1100px; /*utilise "max-width" au lieu de "width" ici et essaie de mettre une valeur superieur a 800px , tu peux même aller jusqu'à 1200px*/
grid-template-columns: 1fr 1fr 1fr 1fr; /*instead of doing 220px 220px 1fr 1fr*/
grid-template-rows: 1fr 1fr; /*instead of using 240px 230px*/
}
Also instead of using the <br> tag to break the lines you can just use display:block on the inline elements
/*You can just remove all the <br> tags from your document and apply these lines*/
.blanc{
display:block
}
.opa{
display:block
}
Use 'grid-row' and 'grid-column' to place your items inside the grid, it will fit best than 'grid-area' for this use case For example:
.part1{
grid-column:1/3
}
.part2{
grid-column:3/4
}
.part4{
grid-row:2
grid-column:1
}
etc....
Try to also add more breakpoints to improve the responsive part
Je t'encourage beaucoup a plus faire des recherches sur 'grid-column' et 'grid-row' plutot que d'utiliser 'grid-area' Et aussi a renommer tes classes en anglais pour avoir plus de feedback et pouvoir évoluer dans ton parcours de developeur front-end
On the mobile nav bar, how do you only darken the background? I tried setting the brightness of the page to 50% but then I could not undo it on the nav bar section as it is a filter.
Nice challenge @JakeHand For a response to the question you asked about darkening the background, you can just add an ::after or ::before pseudo element to your nav and set to it the same rules you have on your mobile nav
nav::after {
content: "";
position: fixed;
width: 100%;
height: 100%;
background-color: rgba(0,0,0,0.4);
transition: 0.4s ease;
display: block;
z-index: -5;
}
Hope this will be helpful🙏
Great challenge @MLH3, One thing i see that can be improved is on the element that got the "section_item" class where you can avoid centering your content vertically, So it can work better on breakpoints
.section_item{
display:flex;
align-items:center;
gap:1rem
}
/*I just delete the justify-content:center line here*/
You can also add more padding to the body element by adding another breakpoint, which will make you much closer to the desktop design For example
@media(min-width:1100px){ body{ padding:40px }
Also on the hero image, instead of using display:none to hide the desktop or the mobile version; You can just use the 'picture' element which will display images based on screen size
<picture> <source srcset="images/image-web-3-desktop.jpg" media="(min-width: 800px)"/> <img class="hero_img" src="images/image-web-3-mobile.jpg" alt="Hero Image"/> </picture>
This is a link if you wanna know more about the 'picture' element: MDN doc
welcome with your feedback
Hi @Alijebbouri, great job But i think to improve responsiveness you could actually add a breakpoint at 900 px so your heading and your paragraph will have more space and that will avoid them to break for example you can do:
@media(max-width: 900px){
.header{
margin-left: 50px;
}
You could also add a margin bottom to your h1 to put it a little bit away from the paragraph
i found the area of screen-size on different screen-types tedious,i'm unsure if i got that right
it's actually nice but i would use a fix width instead of percentage(for example max-width:300px instead of width:20%) But besides that it's all good i think and i fell you having issues with responsiveness of the box but setting a "max-width" will actually help you more with that, and you can either set "width:300px"
