@CitycodSubmitted 3 months ago
Luís Gustavo Gorniak
@luisgustavogorniakAll comments
- @luisgustavogorniakPosted 3 months ago
Nicely done man, looks very similar to the design. Your code is very well written, and the styles look really great! I can see you put a lot of effort in it! Congrats and keep coding!
0 - @JoshdefleurSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
Im proud of the fact I made it responsive, but next time I would like to learn what are the industry standard breakpoints. Also if it was better to use flexbox or css grid. Im not sure when you should use either one
What challenges did you encounter, and how did you overcome them?It was a challenge to get the breakpoints to seemlessly transition. Also it was hard to get my title of the page to line up with 2 lines of equal length, im not sure i did it right because it looks stretched in some cases. But i did research and tried my hardest.
What specific areas of your project would you like help with?Trying to get my title lines to be the same length without stretching so much?
@luisgustavogorniakPosted 3 months agoNicely done! Looks really close to the design! I can see you put a lot of effort in it!
To answer your first question,
flexboxandgridare CSS rules that basically do the same thing, but the best practices are to use grid when its for building a complex layout and flexbox is just for align elements.If you're interested in build a table-like layout,
gridis a go, but if you're trying to align elements inside the cells of the grid created table,flexboxis the best choice.About the responsiveness of the page:
- I see you worried about the UX by adding a lot of media queries for differents viewports of differents type of devices. In this kind of job its better that you keep it simple, a single media query call just for when the viewport is for a mobile device is enough.
- That said, my advice is to adhere to the concept of mobile first. Today is importanter that you first build your code to work on mobile devices first, and then you add the media query for its no longer a screen of a mobile device, like:
@media screen and (min-width: 784px).
You'll see that its much less work and your code will look cleaner.
Lastly, about your titles, they look streched because they're using the same
h1element, and its parent element is setted to have a static width. Try removing the width rule and separating the titles withh1tag for each, and change the parentdivrules forflexboxandtext-align, like this:.bigText { display: flex; flex-direction: column; text-align: center; }I hope my review is helpful, my friend! Have a good one and keep coding!
0 - @Mudassir-CoderSubmitted 3 months ago@luisgustavogorniakPosted 3 months ago
Well done! It looks very close to the design!
There are two points that I consider relevant:
Just be careful when setting the
widthof the elements, depending on the user's viewport, the card will look messy. Instead, try changing it tomin-width, so when the viewport changes, it will look as expected.When working with responsive pages, I recommend that you use the mobile concept first and then adapt it to a larger viewport. You will see that it is much easier to code than the other way around and, after all, people use their mobile devices much more than desktop computers, so to speak.
Other than that your project looks really great! I hope my review is useful! Have a good one and keep coding!
Marked as helpful0 - @MatPawlukSubmitted 4 months ago@luisgustavogorniakPosted 4 months ago
Looks really great man, nicely done!
The desktop version needs a little fixing because of the extra div at the end of the HTML file, but its nothing, If you remove it probably the white line at the bottom of the page will desapear.
The mobile version needs some padding and some alignment in the
maintag, but that's about it!I hope my review was helpful! Have a good one and keep coding!
Marked as helpful0 - @WilliamLelesSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
de ter feito este projeto melhor do que o ultimo
@luisgustavogorniakPosted 4 months agoYour code is very well written! You really explored a lot of HTML semantics. Good use of the
sectionandmaintags.There are some issues with the styling, specifically sizing. The card is kind of breaking when you go into developer mode to adjust the viewer's screen size.
Be careful when setting the
widthandheightof an element. These rules will end up messing up the element if the viewport is different from the one you were developing the code in.Try using
marginandpaddinginstead, the element will be static unless you set a rule for media query, which by the way it would be nice if you added it to differentiate mobile and desktop viewports.Other than that, congrats on the effort and keep coding!
0 - @MouadaselSubmitted 4 months ago
- @BekzodIsakovSubmitted 4 months ago@luisgustavogorniakPosted 4 months ago
Looks good, Nicely done!
Here's some points that could help you improve the code:
- It needs a little bit of work on the margin & padding of the elements of the card, the figma file contains all these information.
- try using some semanthics HTML tags, like
<article>and<section>and CSS rule names,like .cardand.card-text;
I hope my review is helpful! Have a good one and keep coding!
Marked as helpful1 - @luisgustavogorniakSubmitted 4 months agoWhat are you most proud of, and what would you do differently next time?
It was a simple template, just created some CSS rules and it worked well.
What challenges did you encounter, and how did you overcome them?Couldn't find the specs of the fonts, sent the repo anyway.
@luisgustavogorniakPosted 4 months agoThank you so much for your feedbak! I really appreciate it! Did some changes as you suggested, it looks a little closer now. It still needs more work on the margin and padding of the card elements I guess.
0
