@ArseniyX
Posted
Hello, its looks awesome! Design looks incredible so I don't have any suggestions
Code improvements:
- You have hardcoded static data inside HeroSection.jsx and other components
- You have code comment in Illustration.jsx that need to be removed
- in utils you have file name motion.js its just animation utils not necessarily for motion so it need be renamed
- instead of doing {/* prettier-ignore */} you can make svg as component in this case you can put it like independent file .svg in assets
Marked as helpful
@MelvinAguilar
Posted
@ArseniyX Hello! Thank you very much for taking the time to review my solution. I really appreciate your feedback.
- According to you, should all information be generated dynamically? Even the headings, or am I getting confused? Could you provide me with some reference to learn better?
- I just deleted the comment. Thank you.
- I used this project as a reference for the file name. Any suggestion for naming the file? "animation.js"?
- Noted.
Once again, thank you very much for reviewing this solution. It is very helpful and I learn a lot.
@ArseniyX
Posted
@MelvinAguilar
- Well its seems like landing page for company, right? For example if you give/sell this landing and the customer will want to make changes to the content. Its would be better if all content exist inside data.json or something, instead of .js files everywhere. JSON can be modified by no dev oriented person, so it's make it much better for the customer.
- You can called animationUtils.js Always appreciate to give feedback
Marked as helpful
@MelvinAguilar
Posted
@ArseniyX Very comprehensive, thank you again for the wonderful comment. š
@ArseniyX
Posted
@MelvinAguilar btw, you can rewrite getMaxWith
function like that:
const getMaxWidth = (index) =>
["max-w-card-1", "max-w-card-2"][index] || "max-w-card-default"
if you want
Marked as helpful
@MelvinAguilar
Posted
@ArseniyX Thank you, I will update it as soon as possible :)