Samuel Palacios
@samuelpalaciosdevAll comments
- @sirriah@samuelpalaciosdev
Hello, Sirriah 👋
Well done on this challenge!. Your solution looks nice (almost pixel perfect) and it scales pretty well.
I only suggest some things😉:
-
Adding
outline: none
to the buttons with the icons. When I click on them, that outline doesn't look bad but I'd not display it. -
About your sass stylesheets. I suggest you to watch [this video] (https://www.youtube.com/watch?v=9Ld-aOKsEDk&ab_channel=KevinPowell)
I really like the way you approach this, keep coding ;).
-
- @wdluft@samuelpalaciosdev
Hello, William! 👋
Great job on this challenge!. Your solution looks nice (almost pixel perfect) and it scales pretty well.
I only suggest some things😉:
-
Adding
cursor: pointer
to your button. So people would know that's clickable. -
Take care of those accesibility issues. I suggest you to use the [W3C Markup Validation Service] (https://validator.w3.org/).
I really like the way you approach this, keep coding ;)
-
- @spymon@samuelpalaciosdev
Hello, Spymom! 👋
Well done on this challenge!. Your solution looks great and it scales pretty well.
I only suggest a little thing😉:
- I think that those images, don't need an alt text. In this instance, that's a good thing, as the alt text is currently repeating the same as the card headings, so if you take this tip, use
alt="" aria-hidden="true"
on your HTML markup.
I really like the way you approach this, and that fancy animations you added are so cool, keep coding ;)
- I think that those images, don't need an alt text. In this instance, that's a good thing, as the alt text is currently repeating the same as the card headings, so if you take this tip, use
- @ApplePieGiraffe
Invoice App | React, Next.js, styled-components, Formik, Framer Motion
#motion#react#styled-components#next@samuelpalaciosdevHi, ApplePieGiraffe👋
Congratulations, another amazing solution from you (as expected). I love the soft and cool animations you used. Your work impresses me, again, congratulations, mate ;).
Keep coding🧡
- @orandorisk@samuelpalaciosdev
Hi, Orlando👋
Well done on this challenge!👍
I only suggest some things 😉:
-
Be clear with your HTML classes. I don't understand what you're trying to say with example:
flav
orindi
. -
Using
min-widths
andmin-heights
instead explicitwidth
aheight
properties. Setting explicit widths and heights usually is not the best approach, As I'm seeing on your project, on the 900px viewport, one of the containers (the right one) lacks information, if you use min-widths, it will help you.
I hope this would help you, have a nice day, keep coding!💙
-
- @Sven72@samuelpalaciosdev
Hi, Sven👋
Good job on this challenge! Your solution looks good and it scales pretty well.👍
I only suggest some things 😉:
-
Changing the styles for the social media icons. As I see on you project, the twitter icon looks different from the others. That's because you set a
width
on them, I'd delete it, makes them look better. -
Setting a padding on the
main
element. You only have padding for the left & right, but the "Social media dashboard" is too close to the top of the page, I'd addpadding: 2rem 3rem
on the 768px media query.
I hope this would help you, have a nice day, keep coding!💙
-
- @SarahHenriette@samuelpalaciosdev
Hello again, Elise👋
Well done on this challenge! Your solution looks good and it scales pretty well.👍
I only suggest a little thing 😉:
- Decreasing the transition timing when opening the question. I'd change it to .3s, it makes it feels more smoother.
I really like the way you approach this, have a nice day, keep coding!💙
- @akshay63@samuelpalaciosdev
Hi, Akshay 👋
Well done on this challenge! Your solution looks good and it scales pretty well.👍
I only suggest some things 😉:
- Maybe, you don't need height on the cards. I mean, setting an explicit height, could cause you some problems, when talking about responsivenes, you could try to get the same result using padding.
I hope this would help you, have a nice day, keep coding!💙
- @nicole-tuznik@samuelpalaciosdev
Hi, Nicole👋
Well done on this challenge! Your solution looks good and it scales pretty well👍
I only suggest some things 😉:
-
Adding a transition to the toggle. I'd add a
transition: .5s;
on the#toggle::after
element, it make it feel more smooth. -
You don't have a h1 on this project as it stands. Having your headings on order is not really a rule, but the h1 it's one of the most important tags on an webpage . For this project, it would be where you've got the h5 heading.
I hope this would help you, have a nice day, keep coding!💙
-
- @davidtrikic@samuelpalaciosdev
Hi, David👋
Great job on this challenge. Your solution looks good and it scales pretty well👍
I only suggest some things 😉:
-
You don't have a h1 on this project as it stands. Headings provide valuable information by highlighting important topics and the structure of the document. For this project, I'd set it where is the person's name.
-
About the background image, I'd recommend you to use viewport units, especially
vw
in this case, I suggest you to watch this video
I hope this would help you, have a nice day, keep coding!💙
-
- @Gabrieldev-web-coder@samuelpalaciosdev
Hi, ippo👋
Well done on this challenge! Your solution looks good👍
I only suggest some things 😉:
-
You don't have a h1 on this project as it stands. Headings provide valuable information by highlighting important topics and the structure of the document. For this project, I'd set it where is the person's name.
-
There're too many Media Queries on the background position. I'd think you should see others solutions to see how they approach that, because too many media queries are not the best approach I think.
I hope this would help you, have a nice day, keep coding!💙
-
- @tekayafiras@samuelpalaciosdev
Hi👋
Great job on this challenge! Your solution looks good and it scales pretty well. 👍
I only suggest one little thing 😉:
- I think you should change the order of the headings tag. The
h1
tag is usually used for the title of a page, in this case "Social media dashboard", actually is anh2
, instead I'd set the followers number as anh2
.
I hope this would help you, have a nice day, keep coding!💙
- I think you should change the order of the headings tag. The
- @Xhoni43@samuelpalaciosdev
Hi, Xhoni👋
Great job on this challenge. Your solution looks good and it scales pretty well. 👍
I only suggest some things 😉:
-
On mobile viewport the menu isn't centered. As I'm seeing on your project that's because you set a margin-left to the anchor links (
.hero > .navbar > .nav-links > li a
) , I'd delete the margin-left and addalign-items: center;
on theul
which in this case you selected as:.hero > .navbar > .nav-links
. -
Be careful with specificity. I mean, you have extra large selectors like the one I mentioned above, you could simplify that setting a class to those elements and just selecting the class. As an example, adding this class to the anchor link
<a class= nav__link>
, you could select it on CSS as.nav__link
, and it is easier to read than the other selector
I hope this would help you, have a nice day, keep coding!💙
-
- @xuantam-h@samuelpalaciosdev
Hi, Xuan-Tam👋
Great job on this challenge. Your solution looks good and it scales pretty well👍
I only suggest some things 😉:
- You don't have a
h1
on this project as it stands. Having your headings on order is not really a rule, but theh1
it's one of the most important tags on an webpage . For this project, it would be where you've got the h2 heading.
I hope this would help you, have a nice day, keep coding!💙
- You don't have a
- @analuzcervantes@samuelpalaciosdev
Hi, Anna👋
Great job on this challenge. Your solution looks nice and the accordion works well ;)
I really like the way you approach this, have a nice day, keep coding!🧡
- @richardconvery@samuelpalaciosdev
Hi, Richard👋
Great job on this challenge. Your solution looks good and it scales pretty well👍
I only suggest some things 😉:
-
Knows when to use paragraphs insteads ofheadings
h1-h6
. Headings provide valuable information by highlighting important topics and the structure of the document, you should know when to use and when not an heading tag. I'd not set the "followers" as an heading instead as a paragraph. -
About the dark mode question. I suggest you to watch this video.
I hope this would help you, have a nice day, keep coding!💙
-
- @Pedro-Vitor-SG@samuelpalaciosdev
Hi, Pedro👋
Great job on this challenge👍
I only suggest some things 😉:
-
Maybe, you don't need height on the cards. I mean, setting an explicit height, could cause you some problems, when talking about responsivenes, you could try to get the same result using padding. As I'm seeing on your project, the cards on desktop wiewport lacks content, that's because of the height you set.
-
On mobile view, there's some scrollbar. I'd set an
overflow-x:hidden
on thebody
. The content is too big to fit in the specified area on mobile view, setting anoverflow-x:hidden
would help.
I hope this would help you, have a nice day, keep coding!💙
-
- @luibernip@samuelpalaciosdev
Hi, Luis👋
Great job on this challenge. Your solution looks good and it scales pretty well👍
I only suggest some things 😉:
-Maybe checking the HTML Structure I'd set the content as an entire section, not two different. They share relation.
I really like the way you approach this, have a nice day, keep coding!💙