@StevetheRebelSubmitted 3 months ago
What are you most proud of, and what would you do differently next time?
It was an easy task to accomplish. Enjoyed it very much.
Steven Noyes
@SteveNoyesAll comments
- @SteveNoyesPosted 3 months ago
Your code is well-structured and adheres to good practices, especially with the use of variables and Flexbox. Minor adjustments, like enhancing accessibility, refining media queries, and ensuring better responsiveness across various devices, will improve the overall quality and maintainability of the code.
The header section is well-styled, but using gap: 1rem and height: 30vh might create layout issues on very small screens. Consider using min-height instead of height to ensure the content doesn’t overflow on smaller screens.
The main section and its child elements use Flexbox, which is good for responsive design. However, using a fixed width like 30% for .col-ctn may cause layout issues on different screen sizes. Use flex-basis with min-width or max-width properties instead of a fixed width: 30% to allow better flexibility across screen sizes.
The box-shadow on the cards (2px 2px 10px var(--neutral-grayish-blue)) is subtle and may not be noticeable on certain backgrounds. Consider increasing the blur radius or opacity for a more pronounced effect, or adding a background color to the body or container.
The media query at max-width: 500px is a good start, but it might not cover all smaller devices or edge cases. Add another breakpoint, perhaps at max-width: 768px, to better handle tablet or small laptop screens.
The images have a fixed width of 2.5rem, which might not scale well on larger or smaller screens. Use responsive units like em, rem, or percentages, and consider max-width: 100% to ensure proper scaling.
Marked as helpful0 - @jeimosSubmitted 3 months ago@SteveNoyesPosted 3 months ago
Your code looks great, it is concise and well organized. There are a few suggestions below.
The global reset * { padding: 0; margin: 0; } is good for eliminating default browser styles, but you might want to include box-sizing: border-box; for better control over element sizing. Consider adding box-sizing: border-box; globally or for specific elements where necessary.
The .container class uses margin-left: 450px;, which is not responsive and may cause layout issues on smaller screens or different resolutions. Replace margin-left with margin: 0 auto; for better centering. You can adjust with media queries if necessary.
The .btn class could use padding values that are more dynamic, as the current values may not adjust well to content changes or translations. Use relative units (e.g., em or rem) or percentages for padding to ensure better scalability.
The .footer and .attribution have identical styling in both the main and media query sections. This repetition can be avoided to streamline the code. Consolidate the shared styles outside the media queries to avoid redundancy.
The media query is responsive, but the use of fixed widths like 375px and 290px might not work perfectly on all mobile devices. Consider using percentages or vw (viewport width) units to make the layout more adaptable to different screen sizes.
0 - @franciscolima-proSubmitted 3 months ago@SteveNoyesPosted 3 months ago
You're using a percentage for the .container width but fixed pixel widths in media queries. It’s better to stick to one unit type for consistency and responsiveness. You can use max-width in combination with percentages to ensure flexibility across different screen sizes.
The class name .conteiner appears to be a typo. It should be corrected to .container to follow standard naming conventions.
The font family is defined both in the body selector and within specific heading tags. Since a consistent font style across the entire site is often desired, it’s best to apply the font families in :root or at the body level and override it only when necessary.
0 - @fivetailsdevelopmentSubmitted 3 months agoWhat are you most proud of, and what would you do differently next time?
Made better use of EM and REM for sizing elements appropriately. Will look into using functionality like clamp to handle responsive design container sizing more effectively.
What challenges did you encounter, and how did you overcome them?It was harder to get the sizing close without the Figma file but made for a fun challenge.
@SteveNoyesPosted 3 months agoConsider using rem or em instead of px for more flexible and scalable units, particularly for the font-size, padding, and margin values in media queries and other places.
0 - @Griever9918Submitted 3 months ago@SteveNoyesPosted 3 months ago
You code is structured well, the root variables are great, especially the font weight.
Marked as helpful0 - @Vaibhav-Kumar-K-RSubmitted 3 months ago@SteveNoyesPosted 3 months ago
This looks great. I have three things I think could help improve your design.
The font is not pulling through for the h1 and p elements. While the font is set within the 'roboto-regular' class, it is overwritten by the 'attribution' class with sans-serif.
Adding line-height to the card element will give the h1 and p a little more space aligning closer to design.
And adding a bit more width (and height) to the card element.
Marked as helpful0 - @MishywayuSubmitted almost 2 years ago
I found it difficult to deploy the project on GitHub pages. Though I managed my way through. However, once I click the live site URL, the image is not displayed in the browser. I have tried looking for the solution to it but found none. Anybody help please.
@SteveNoyesPosted almost 2 years agoIf you change your background image in your css it will show your image. The file path just needs a period. background-image: url(./image-qr-code.png); CSS Line: 22
0 - @virag-kySubmitted about 2 years ago@SteveNoyesPosted about 2 years ago
This looks great! I believe if you wrap your div in a main tag it will take care of the accessibility issue.
0 - @mohamed-benoughideneSubmitted about 2 years ago
