Amir Jacobs has commented on Guliye's "Responsive FAQ accordion using JS" solution2Hi GuliyeI noticed a few things about your Javascript:I'd use functions to make clear what is happening. You could make a function called resetAccordion() and a function called showAccordion().I'd make a bit more use of comments.The accordion can only be opened by clicking the icon, but it'd be more user-friendly to have it open upon clicking .question.Have a nice day. ✌
Amir Jacobs has commented on Rodrigo Gamboa's "Profile Card Component using HTML and CSS" solution3Hey Rodrigo,I've taken a quick look and I noticed the following:Instead of width: 375px I'd use width: 100%; max-width: 375px as this helps with responsive.The blue background pattern images don't show up on mobile.I'd add a margin to the sides of the page on mobile, so that the card doesn't touch the edges. Something like 5px to 15px should be perfect.You could use the BEM method for your CSS. Not really necessary here as it's pretty clear, but I'd look into it nonetheless.Your code looks clean. Have a nice day. 😊
Amir Jacobs has commented on Bryan Martins de Araujo's "Tracking Introduction" solution0Hey BryanI've taken a look and I noticed the following things:Your responsive design (376px and up) has some flaws. You tend to use > in your CSS. I would make the elements specific by utilizing the BEM method.You're using CSS variables -- that's great.Happy coding 😄
Amir Jacobs has commented on Karim's "huddle-landing-page-with-curved-sections" solution2Hi KarimI took a quick look and I have some suggestions:I'd stop using <br> to create space between elements. Instead, use margin or padding.Your indentation could be improved -- perhaps look at how a code beautifier would indent your code.In some cases I see inline-styling (such as <h1 style="font-size: 65px">2.7m+</h1>. I would put all styling in your CSS file instead.Your class names and id's could be more descriptive. It's difficult to immediately tell what #final or #final2 is.You also have some html and accessibility issues (there's a report of it on this page). You're on the right track. 😄
Amir Jacobs has commented on Vanessa Martin's "Clock App Using React + CSS (Flexbox, Web-First Solution)" solution1Looks great! I love that you put the CSS files with the corresponding component -- before this I'd have it under ./src/css.The only thing I'd change is I'd use functional components.
Latest Comments
Amir Jacobs has commented on Guliye's "Responsive FAQ accordion using JS" solution
Hi Guliye
I noticed a few things about your Javascript:
resetAccordion()
and a function calledshowAccordion()
..question
.Have a nice day. ✌
Amir Jacobs has commented on Rodrigo Gamboa's "Profile Card Component using HTML and CSS" solution
Hey Rodrigo,
I've taken a quick look and I noticed the following:
width: 375px
I'd usewidth: 100%; max-width: 375px
as this helps with responsive.5px
to15px
should be perfect.Your code looks clean. Have a nice day. 😊
Amir Jacobs has commented on Bryan Martins de Araujo's "Tracking Introduction" solution
Hey Bryan
I've taken a look and I noticed the following things:
>
in your CSS. I would make the elements specific by utilizing the BEM method.Happy coding 😄
Amir Jacobs has commented on Karim's "huddle-landing-page-with-curved-sections" solution
Hi Karim
I took a quick look and I have some suggestions:
<br>
to create space between elements. Instead, usemargin
orpadding
.<h1 style="font-size: 65px">2.7m+</h1>
. I would put all styling in your CSS file instead.#final
or#final2
is.You're on the right track. 😄
Amir Jacobs has commented on Vanessa Martin's "Clock App Using React + CSS (Flexbox, Web-First Solution)" solution
Looks great! I love that you put the CSS files with the corresponding component -- before this I'd have it under
./src/css
.The only thing I'd change is I'd use functional components.