If you have any tips or observations, I'm all hearing :)
Joel
@joelsalmeidaAll comments
- @thbdmttSubmitted about 3 years ago@joelsalmeidaPosted about 3 years ago
Hello Thibaud. I have a tip.
Try forming three columns by adding the two middle boxes inside another element "e.g. a flexbox div". Then just adjust the spacing.
It's a simple way to get closer to the original design... without changing too much what you've built.
Hope this helps. Keep coding!
Marked as helpful0 - @12afaelPereiraSubmitted about 3 years ago
Any feedback is appreciated!
@joelsalmeidaPosted about 3 years agoGood job Rafael. I have a tip to make everything even better.
The text of the button does not appear centered. How about adding a single value to the entire padding of the button?
e.g.
padding: 1em;
You can also achieve the optimal size by adding horizontal and vertical padding. "Using only two values. So you don't need to use
width
."e.g.
padding: 1em 3em;
"1em vertically and 3em horizontally"Hope this helps. Keep coding! ;)
Marked as helpful1 - @by-yeeSubmitted over 3 years ago
My 2nd challenge by using React. This challenge let me understand more on the range slider, both Chrome and Mozilla have different implementation when it comes to the range progress bar.
I've added some animation to the price changes and also the button. Any feedback is appreciated. :)
@joelsalmeidaPosted about 3 years agoAmazing! I loved the animations. I can't wait to start diving into React. :)
0 - @13GroszySubmitted over 3 years ago
Do I not overthink some of the code? Could I write it easier and more responsive? What to do with small resolutions under 300px? The website looks terrible if I have to use everything provided without hiding some stuff.
@joelsalmeidaPosted about 3 years agoHello Mateusz!
I believe you could use just one class for all FAQ questions. "Since they have the same formatting"
It would also be nice to try to bring the proportions closer to the original design. This is very important.
Hope this helps. Keep coding. ;)
Marked as helpful0 - @casbernSubmitted over 3 years ago
What do you guys think I need to improve in my code?
For some reason I have difficulties using the semantics on HTML, I would like to know what could I do to make my code more semantic? In this case where I had only the cards, what tags I should have implemented?
Any feedbacks will be welcome!
Thanks you very much.
@joelsalmeidaPosted over 3 years agoHello Cassia.
In this case, I think grouping all the cards inside a
<section>
tag would be the most appropriate. "Since there is no specific tag for cards."You used the
<main>
tag. I don't see a problem in this case. However, as this is only a component,<section>
would be more appropriate.Another semantic tag you could use in this project would be
<button>
instead of<a>
.Important tip: You should change your
<h1>
tags to<h2>
"depends on the context". Because within any page, there must be only one<h1>
tag.Hope this helps. Greetings from Brazil. :)
0 - @ECCERSubmitted over 3 years ago
My third challenge posted, as always all feedback is welcome!
@joelsalmeidaPosted over 3 years agoYou found an interesting solution to this challenge. Good work. Just a quick tip: You can add borders separately. e.g.
border-top
. I believe it would be simpler in this case.0 - @DavidMaillardSubmitted over 3 years ago
Feedbacks are welcome !!
🤘🤘🤘
@joelsalmeidaPosted over 3 years agoHi David. You were really true to the original design.
I also liked the responsive solution. So smooth. Good work.
Marked as helpful0 - @ericpatricioSubmitted over 3 years ago
Feedback!
@joelsalmeidaPosted over 3 years agoHi Eric. Looks like you had a lot of work on this project. Congratulations. I have some tips to make everything even better.
-
You can use header, nav, main, footer, button... tags instead of just naming classes. You can put all sessions inside a main tag.
-
Don't forget the
alt=" "
tags in the images too. -
In the original design the entire page follows a lateral alignment. Check the position of the header in relation to the rest of the body.
-
Check the
font-size
of your paragraphs. Some are too small. -
Check the title color of articles on hover.
-
When opening the "mobile" hamburger menu the page should not be able to scroll. "I liked the animation, so smooth :)"
Hope this helps. Keep coding.
Marked as helpful3 -
- @DanielOsw22Submitted over 3 years ago
I'm having two issues that I cound't really figure out. The first one is, why or how do I make all pictures the same size while keeping it responsive on different container sizes? I think using fixed sizing on width wouldn't be the best option and %'s are relative to the container size. Second one is, using padding on grid items just messes all the grid alignments and I coudn't find a way the fix it.
Can anyone help me with those two issues please?
@joelsalmeidaPosted over 3 years agoHi Daniel. I took a look at your code and I'll give you some tips.
In the original design, all cards have the same spacing pattern.
-
You can achieve this result by applying
column-gap
androw-gap
to the container. "no percentages, tryem
orpx
...". Four columns and two rows. All have the same fraction in this case 1fr. -
The testimonies must take up all the space. Just add some
padding
around it. Then just work on the text spacing. -
About the pictures: You are using disproportionate measures:
max-height: 50%;
max-width: 100%;
Try something like:width: 50px;
height: auto;
"keeping proportion".
Hope this helps. Keep coding. ;)
Marked as helpful1 -
- @Developer3027Submitted over 3 years ago
I did not download to kit. This is built off the concept of the images. I used a image off unsplash. It was after I worked it up that I realized you can download the kit, however, I like this concept so this is what I am submitting. Surely I can not be the only one to have made this mistake. I set it up in a aws s3 bucket for review. When this form failed due to the s3 bucket http, I set up in Vercel. Hope you enjoy. Please comment.
@joelsalmeidaPosted over 3 years agoI recommend that you try it with "starter files" too. It's pretty fun. "And closer to real work" You will also have access to a guide on how to get started and publish your solution. Keep coding. ;)
1 - @VictorrrochaSubmitted over 3 years ago
To give the image that tint, I created a div with transparency on top of the image. Don't know if that is the best method to do this, but I only used vanilla HTML/CSS. If anyone has a better idea, please tell.
I would love to get some constructive criticism on working with images.
@joelsalmeidaPosted over 3 years ago-You can use
mix-blend-mode
to get this effect. "I think overlay would be cool in this case."-You can keep the aspect ratio of the images using
width: 100%; height: auto;
. "vice versa"-Try to plan your classes so that you only use what you need. "Your code will be easier to work with."
Hope this helps. Keep coding.
Marked as helpful1 - @vanzasetiaSubmitted over 3 years ago
Hello Everyone! 👋
I was doing this challenge on my Android phone. Hopefully, it looks good on your deskop. 😅
Of course, any feedback is appreciated!
That's it! Happy coding everyone!
@joelsalmeidaPosted over 3 years agoIf you didn't say you do it on your smartphone... no one would know.
I had problems trying to use
@use
instead of@import
. Looks all right on your project.Absolutely a good job. Looks good on all screen sizes.
0