Agustina
@aguscorvoAll comments
- @Mina546845414h55gSubmitted 3 months ago@aguscorvoPosted 2 months ago
Congrats on finishing the challenge!
Here are some suggestions or tips to improve the solution:
- How about using semantic HTML? I noticed that the main tag is missing, for example.
- The purpose of the alt property is to describe the image.
- Some text is using the wrong font family, such as the title, price, or button.
- "Perfume" should be in uppercase.
- Responsive is working properly! However, be aware that the image borders change in the mobile view.
- I noticed a few typos in the CSS classes, or they may be incorrectly named (for example, using "buttons" for the price div). While this may not be an issue now, it could be confusing when you revisit the code, especially when working in a team.
Happy coding! :)
Marked as helpful0 - @jen067Submitted 3 months ago@aguscorvoPosted 3 months ago
Congrats on completing the challenge!
Here are some suggestions or tips to improve the code:
- Great use of semantic HTML! How about changing the div tag with the intro-content class to a section tag?
- I think you can remove the only div inside the main tag and adjust the main styles accordingly.
- Delete the comments in the HTML file.
- There are several colors mentioned in the style guide that are missing in the solution.
- Instead of adding margins to certain elements, you can use the gap property.
- Minor suggestion: Adjust the horizontal padding so that the text fits the design.
- Add some content to the README file.
Happy coding!
0 - @crimson3dSubmitted 10 months ago@aguscorvoPosted 3 months ago
Congrats on solving the challenge! Here are some suggestions or tips:
- Include semantic HTML tags like main, section, and article when necessary, instead of using divs for everything.
- There’s something missing in the hover state: while the background color changes, the text color stays white, which makes it hard to read. According to the designs, it should be black.
- From my understanding, "GitHub", "Frontend Mentor", etc, should be buttons.
- Explore other ways to manage spacing between elements instead of using <br> tags. For example, you can use the gap property with flexbox.
- Please check the size of the avatar image, the card size and the padding. Also, note that they may change slightly in mobile view.
Happy coding! :)
0 - @franklobsty25Submitted 3 months agoWhat are you most proud of, and what would you do differently next time?
I'm proud of the challenge. What to do differently is the rounded corners of the image which I found challenging.
What challenges did you encounter, and how did you overcome them?The rounded corners of the SVG were the challenge for which I couldn't find a solution yet. I need help with that or resources that couldn't assist me.
What specific areas of your project would you like help with?SVG rounded corners on the challenge design, which I couldn't resolve.
@aguscorvoPosted 3 months agoGood job on solving the challenge! I saw that you already fixed the rounded corners on the illustration image—well done!
Here are some suggestions to improve the solution or tips moving forward:
- The title is missing a hover state.
- How about changing the p tag with the class title to an h1 tag?
- Instead of adjusting margins or using unnecessary padding on each element, you could use display: flex and gap. This approach also makes it easier to match the designs. When you start styling components, you’ll notice the benefits of this.
- For future projects, if you have the chance to check the Figma designs, take advantage of it to ensure the sizes and other properties match.
- I’m following the “Getting started on Frontend Mentor” path, and there’s an interesting challenge: “The font sizes in this project are slightly smaller in the mobile layout. Find a way to reduce the font size for smaller screens without using media queries.” It’s fun to solve!
- One more tip for future projects: folders that start with a dot, like .vscode, are generally not meant to be included in the repo. I suggest adding them to the .gitignore file to avoid issues when working with other developers.
Marked as helpful0 - @nicolaee2Submitted 3 months ago@aguscorvoPosted 3 months ago
It looks great! Here are some suggestions:
- Positioning the elements inside the container using flexbox, it's easier and more readable
- Use an article tag instead of a div with class "text"
- Use a heading tag instead of a p with class "title"
- Add a box shadow to the card
Lastly, I think the repo link is broken.
Happy coding :)
Marked as helpful1