Guilherme Dias Simões
@gdsimoesAll comments
- @ImBenjaSubmitted 6 months ago@gdsimoesPosted 5 months ago
Your solution looks really good; however, there are some minor inconsistencies with the design:
- The image should have a width of 50% of the entire card.
- The word "CHANEL" is a different color than the rest of the paragraph.
- The button is not aligned with the text above.
If you address these issues, I'm sure your project will be even better!
0 - @MunibAhmad-devSubmitted 5 months agoWhat specific areas of your project would you like help with?
For the nutrition section below, I struggled to get it right, and it still isn't that good. Maybe it's because I didn't use a grid. I always use flexbox for everything.
@gdsimoesPosted 5 months agoI don't think Grid would help here. You should have used a table element since this is tabular data. If you need help learning how to style tables I recommend checking out Kevin Powell's video about this topic on YouTube.
Also, have you checked the style guide for the colors? It seems like yours are a bit off.
Marked as helpful1 - @porumbachanovSubmitted 6 months agoWhat are you most proud of, and what would you do differently next time?
Nothing in particular, would probably try a smarter way to do the css.
What challenges did you encounter, and how did you overcome them?It was pretty similar to the Blog preview card.
What specific areas of your project would you like help with?Maybe a better way to write the responsiveness and css as whole.
@gdsimoesPosted 6 months agoI really enjoyed reviewing your project—it's clear you've put a lot of effort and creativity into it! The overall design is visually appealing, and the functionality seems robust. I do have a few suggestions that could enhance the usability and accessibility of your project even further.
-
Contrast Issue: The contrast in your footer links is currently too low, which may cause accessibility issues for visually impaired users. Consider enhancing the contrast to improve readability.
-
Font Size Units: It's recommended to avoid using pixels for setting font sizes, as this can restrict users who need to adjust font sizes in their browser settings for better readability. Consider using relative units like
em
orrem
instead. -
Layout Consistency: The spacing between the description and the first button differs from the original design. Adjusting this distance could enhance the visual consistency of your project.
Marked as helpful1 -
- @thebigfatpaandaSubmitted 7 months agoWhat are you most proud of, and what would you do differently next time?
that is completed the challenge i would try to make the website responsive from the beginning.
What challenges did you encounter, and how did you overcome them?i faced problems in positioning some of the components and also faced issues in making it responsive
What specific areas of your project would you like help with?I would like to get help in positioning and responsiveness thankyou.
@gdsimoesPosted 7 months agoDesign & Layout:
-
Image Alignment: The main image appears off-center, which affects the visual balance of the page. Consider adjusting the CSS properties like
margin
to center the image effectively. -
Card Width: The card is currently too wide.
-
Scrolling Issue: On smaller screens like laptops, it would be better if the entire page fit without needing to scroll. This can be achieved by making sure the card height is equal to the one in the design.
Code Quality:
-
Code Formatting: There are some inconsistencies in your formatting. Tools like
Prettier
can automatically format your code, making it cleaner and more readable. -
Naming Conventions: Using IDs like
div1
andimg2
can make the code harder to maintain. More descriptive class names would enhance the readability and reusability of your CSS. For example, using.profile-card
instead of#div1
could be more descriptive. -
Use of
<br>
: Instead of using<br>
tags for adding space, it's better to use CSS properties such asmargin
orpadding
. This approach is more flexible and keeps your HTML cleaner.
General:
- Your choice of colors and fonts aligns well with the design file, which is a great start! With some adjustments to layout and code structure, your project will be even better.
Marked as helpful0 -
- @gdsimoesSubmitted almost 3 years agoWhat are you most proud of, and what would you do differently next time?
I finished this project a long time ago, so I don't feel particularly proud. Next time I will use Prettier for formatting my code and Josh Comeau's CSS reset instead of the CSS remedy.
What challenges did you encounter, and how did you overcome them?I don't remember any.
What specific areas of your project would you like help with?This project for me was mainly about learning the workflow of submitting solutions and so on. I don't think I need any help in this one.
- @JoramirJrSubmitted 8 months agoWhat are you most proud of, and what would you do differently next time?
Well, it was a very simple problem, so I don't think there's much room for improvement. Overall, I'd say that I could keep an eye on accessibility principles, since I'm aware of none of them. I'm not sure if I used the correct HTML semantic tags for the job, so that would, maybe, be something I'd change.
What challenges did you encounter, and how did you overcome them?None
What specific areas of your project would you like help with?I used a tag for the bottom text, but I'm not sure if that was a good decision. Also, In order to align the QR code image, I used a wrapping for the job, but probably I could've centered the , in the top section, without using an extra .
@gdsimoesPosted 8 months agoIt looks pretty good and if you use the
box-shadow
it will look even better!Also, you might consider using a separate CSS file. It won't change the final website but makes for a better developer experience.
2 - @BrunoAgazziSubmitted over 2 years ago@gdsimoesPosted over 2 years ago
Your website looks like the design, but I would like to address a few problems to help you improve it.
I don't know if you are aware, but there is an accessibility report here on Frontend Mentor, and your solution has 13 accessibility issues. So, try to look into that first.
Also, the highlighted day should be today (in my case, a Friday), not Wednesday. To do that, search for the "getDay" function.
When you hover an individual bar, you get the tooltip with the correct amount, but the width of the "bar-container" increases, making the other elements move a little bit. To fix that, you should make the tooltip have the "position" property with the value "absolute." That way, it would not take part in the normal document flow, and your bars would remain static.
Finally, you implemented both the mobile and desktop designs, but as the screen gets larger, the size of your page remains the same. Ideally, it would grow or shrink according to the user's screen.
1 - @lucasnogueiracorreaSubmitted over 2 years ago@gdsimoesPosted over 2 years ago
Your solution looks excellent! Just make sure to always look at the accessibility report, and your project will be even better.
Also, if you want to center things, you could make your body element a flexbox. It will also help if you're going to make your website responsive.
And finally, write a README file. It really helps people who run into your project on GitHub.
Marked as helpful0