Latest solutions
Latest comments
- @cole319P@SimonCassan
The font size looks a bit too small, and the gaps in your flexboxes don’t quite match the Figma mockup. Looks good otherwise.
Marked as helpful - P@oryanhachWhat are you most proud of, and what would you do differently next time?
I am proud of myself for figuring out how to add the modal using JS. I did not do the mobile modal, and the tablet and dekstop's modals displayed properly on fixed sizes, but It was not my main goal in this challenge. Overall, I am proud of myself, and while I made many mistakes during the code, I feel like I took what I needed from this challenge, and will build on those mistakes on future challenges.
P@SimonCassanHTML
-
An article needs a heading to be considered valid. You could replace your paragraph "Share" with an <h2>, for example.
-
Consider adding an aria-hidden to your <div class="bottom-arrow"></div> for accessibility purposes.
-
Your article should be inside your footer so that you can position it absolutely relative to your share icon and make it responsive.
CSS
- cursor: pointer should be applied to .modal__img, not the entire card.
JS
- Adding a class that you style in your CSS makes more sense than using inline styles repeatedly.
Marked as helpful -
- @bennk0509P@SimonCassan
Hello,
You forgot to include the quotation marks in the background of the first card. The box shadow is also missing on those cards, and you didn’t replace the placeholder text in the title and the attribution block.
Additionally, avoid putting styles directly in the HTML — use a linked stylesheet instead. You should also try to include the font directly in your CSS rather than linking to Google Fonts. This improves performance and enhances user privacy.
Good job on the use of CSS Grid!
- P@VirginiaPatWhat are you most proud of, and what would you do differently next time?
I enjoyed building this project. For the first time I used css variables and BEM methodology for the class names.
What specific areas of your project would you like help with?You are welcome to leave any feedback!
P@SimonCassanThe minimum screen width you should support for the mobile layout is 320px.
While calc() is a useful tool, it’s best to use it sparingly to maintain clean, readable, and maintainable code. Overcomplicating layouts with too many calculations could make the CSS harder to manage, especially in larger projects.
Your tablet design looks great—well done!
- P@Oliko136P@SimonCassan
Feedback
-
For your CSS reset, you can directly use the following rule:
*, ::before, ::after { margin: 0; padding: 0; box-sizing: inherit; }
-
Never combine max-width and min-width in your media queries. You need to choose only one.
-
You should apply a max-width to your main content with fixed margins and then center it using Flexbox. This will improve the appearance on medium-sized screens and allow for smoother expansion on larger screens.
Marked as helpful -
- @internetcode999P@SimonCassan
Great job! The card looks good, but it appears a bit too small on larger screens. Instead of using width/min-width, consider applying a max-width with some margin-left and margin-right to ensure there’s free space on smaller screen-sizes.
Happy coding! 😊