Huy Phan• 1,420
@huyphan2210
Posted
Hi, Chrision Wynaar,
I reviewed your solution, and here are a few of my thoughts:
- I noticed that in both this solution and your Blog Preview Card solution, the section with the class
.all-contentis the only child of the<body>element. To simplify, consider applying the.all-contentclass directly to the<body>and removing the<section>tag. This reduces unnecessary markup, making the structure cleaner. - The
.all-contentclass currently haswidth: 100%andmin-height: 100vh. However,width: 100%isn’t needed because<section>(and the<body>, if you apply the class there) is a block-level element, which automatically takes up the full width of its parent by default. It’s still true even if you move the class to the<body>. It’s better to applymin-height: 100vhto the<body>to ensure the content always covers the full viewport height, especially for pages with less content. - Your
<body>doesn’t include any of the semantic tags like<header>,<main>, or<footer>. It’s good practice to use these tags for accessibility and semantic clarity. You could apply the.all-contentclass to the<body>, as I mentioned earlier, and change the.containerto a<main>element instead of a<div>. Similarly, your.attributionsection could be a<footer>instead of a<div>. These semantic tags help screen readers and search engines better understand the structure and purpose of each section. - You mentioned the layout doesn’t match the design, but you didn’t specify what’s off. I assume you’re referring to the width of the summary. You’ve set the
.containerwidth to 35%, meaning it will take up 35% of its parent’s width (in this case,.all-content, which spans the full viewport). The issue arises when the viewport is too small, causing the.containerto shrink and the content to overlap or overflow. I noticed you have a media query for viewports 600px wide and below. Consider adding more media queries for larger viewports or setting amin-widthon the.containerto prevent it from shrinking too much.
Hope this helps!
Marked as helpful
0
