Latest solutions
- Submitted 17 days ago
Tech book club landing page
#accessibility#sass/scss- HTML
- CSS
I'm always open for feedback!
- Submitted about 1 month ago
Project tracking intro component
#accessibility#sass/scss- HTML
- CSS
- JS
As always, open for any feedback.
- Submitted 2 months ago
Skilled E-learning Landing Page
#accessibility#sass/scss- HTML
- CSS
As always, I'm open for suggestions!
- Submitted 3 months ago
Space Tourism multi-page Project
#accessibility#sass/scss- HTML
- CSS
- JS
I'm open to any feedback.
- Submitted 3 months ago
Easybank Landing Page using :has & Subgrid
#accessibility#sass/scss- HTML
- CSS
- JS
As always I'm open for any feedback :)
Latest comments
- @IvanFan-VanSubmitted 4 days agoP@Islandstone89Posted 4 days ago
Hi, good job!
Here is some feedback - I hope it helps :)
HTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify a page's "main" content. Change.container
from a<div>
to a<main>
. -
The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website."
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
Use the style guide to find the correct
font-family
, and remember to specify fallback fonts, in case the font doesn't load on the user's device:font-family: 'Outfit', system-ui, sans-serif;
. -
On the
body
, changeheight
tomin-height: 100svh
- this way, the content will not get cut off if it grows beneath the viewport. -
Descendant selectors like
.wrapper p
increase specificity, making the styles harder to override. It's best practice to instead give elements a class, and use that as the selector. -
.container
doesn't need any styles, so move those properties to.wrapper
. -
Remove
margin-inline: auto
on the card, as it is already centered using Flexbox. I would also remove Flexbox/Grid, as the elements stack on top of each other by default. -
max-width
on the card should be in rem. Around20rem
works well. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
On the image, add
display: block
and changewidth
tomax-width: 100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container.
0 -
- @fecroceSubmitted 4 days agoWhat are you most proud of, and what would you do differently next time?
I created this project at a time when I was getting back into studying the subject, and everything still felt very tangled in my mind. When I finished, I was proud of everything, but especially of my ability to ask for help in the right way to get my questions clarified.
What challenges did you encounter, and how did you overcome them?Everything. As I mentioned before, it was all still new to me — countless questions, especially when building the structure of the 'divs.'
What specific areas of your project would you like help with?CSS is something I still have many questions about, especially how to use it in the best way. It's something I'm continuously working on and improving.
P@Islandstone89Posted 4 days agoHi, well done finishing this first challenge!
I have gone through your code, and here are my suggestions. I hope they are clear and helpful, and that you don't feel disheartened - look at this as a great way to learn!
For maximum results, I highly recommend applying these changes to your solution. You don't need to re-upload the solution - you'll update the repository, which this solution is already connected to.
Good luck :)
HTML:
-
I would remove the first
<div>
- you only need a<div>
holding the card content inside a<main>
. -
The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website." You don't need to include words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code".
-
I would change the heading to a
<h2>
- a page should only have one<h1>
, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components.
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
box-sizing: border-box
is usually set on all elements with the universal selector, like so:
*, *::before, *::after { box-sizing: border-box; }
-
Whenever a value is zero, you don't need to include an unit - for example,
margin: 0px
is usually written asmargin: 0
. -
Since the font is the same for all elements, it's most efficient to set
font-family
on thebody>
, and remove it elsewhere. The descendants of the<body>
will inherit the font, meaning there is no need to explicitly set the font on specific elements. -
Move the styles on
.container
tobody
. -
On the
body
, remove themin-height
inpx
, and changeheight
tomin-height: 100svh
- this way, the content will not get cut off if it grows beneath the viewport. Also, removewidth: 100%
- most elements are block elements, which take up the full width by default. -
Descendant selectors like
.card-content > p
increase specificity, making the styles harder to override. It's best practice to instead give elements a class, and use that as the selector. -
On the card, remove
width: 100%
, as a<div>
is a block element. Also, removemargin: auto
- the card is already centered using Flexbox. -
Remove all widths and heights in
px
. We rarely want to give a component a fixed size, as we need it to grow and shrink according to the screen size. -
max-width
on the card should be in rem. Around20rem
works well. -
Great job having
max-width: 100%
on the image! It's also common to adddisplay: block
andheight: auto
to images. -
As the design doesn't change, there is no need for any media queries. When you do need them, they should be in
rem
orem
, notpx
. Also, it is common practice to do mobile styles first and use media queries for larger screens.
1 -
- @israel4uSubmitted 6 days agoWhat are you most proud of, and what would you do differently next time?
The challenge reinforced my knowledge of flexbox layout. I learned to use a property in a more interesting way than I knew how before. I would need more support in the Grid and flexbox layout. I need to use them more frequently.
What challenges did you encounter, and how did you overcome them?To center the entire content in the middle of the page took me some time. I had to look up some tips on https://developer.mozilla.org before getting it. I felt delighted figuring it out eventually.
P@Islandstone89Posted 6 days agoHi, well done.
Here is some feedback:
HTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify a page's "main" content. Wrap the card in a<main>
. -
The alt text should be written naturally, without using
-
between the words. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website." -
Change
.attribution
to a<footer>
, and use<p>
for the text inside.
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
Since "Outfit" is a sans-serif font, you should use
sans-serif
as a fallback font too. Changefont-family: "Outfit", serif;
tofont-family: "Outfit", sans-serif;
. -
Remove the margin on the card. Instead, add
align-items: center
on thebody
. I would also addgap: 1rem
, and removemargin-top
on.attribution
. -
Remove
max-width: 253px
on. qr-text-content
. -
Remove the
width
inpx
on the image. We rarely want to give a component a fixed size, as we need it to grow and shrink according to the screen size. -
We do want to limit the width of the card, so it doesn't get too wide on larger screens. To solve this issue, give the card a
max-width
of around20rem
. -
font-size
must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
On the image, add
display: block
,height: auto
andmax-width: 100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container.
Marked as helpful0 -
- @Elvys-cSubmitted 6 days agoWhat are you most proud of, and what would you do differently next time?
having completed the challenge, about doing it differently, trying other ways of centralizing the element
What challenges did you encounter, and how did you overcome them?apply white border between qr code image, using padding on the element that surrounds the image tag
What specific areas of your project would you like help with?No help was needed for this one
P@Islandstone89Posted 6 days agoHTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify a page's "main" content. Change.container
to a<main>
. -
The alt text should be written naturally, without using
-
between the words. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website." -
It's best practice to give elements a class instead of an
id
. Here is an article explaining the use cases for theid
attribute. -
"Improve your front-end skills by buildig projects" is a heading. I would make it a
<h2>
- a page should only have one<h1>
, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components.
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
Move
font-family
tobody
, and remember to specify fallback fonts, in case the font doesn't load on the user's device:font-family: 'Outfit', system-ui, sans-serif;
. -
On
.container
, changeheight
tomin-height: 100svh
- this way, the content will not get cut off if it grows beneath the viewport. -
Remove all widths and heights in
px
. We rarely want to give a component a fixed size, as we need it to grow and shrink according to the screen size. -
We do want to limit the width of the card, so it doesn't get too wide on larger screens. To solve this issue, give the card a
max-width
of around20rem
. -
font-size
must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
Paragraphs have a default value of
font-weight: 400
, so there is no need to declare it. -
I would increase the
padding
on the card to around16px
. -
On the image, add
display: block
,height: auto
andmax-width: 100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container.
0 -
- @Aaquino8991Submitted 9 days agoWhat are you most proud of, and what would you do differently next time?
I am proud with being able to mimic the screenshot of the qr code with css. I used to struggle a lot trying to simple stuff like centering, but now I feel like I have a good understanding to be able to mimic another page. I want to organize my css more and structure my html more semantically, implementing more best practice instead of just improvising.
What challenges did you encounter, and how did you overcome them?Converting px to rem. I found a good source on Youtube that explained how to do this. (WebDev Simpilified)
What specific areas of your project would you like help with?Understanding what a CSS Reset is and how to set it up.
P@Islandstone89Posted 9 days agoHi Anthony, good job. Here are some suggestions:
HTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify a page's "main" content. Wrap the card in a<main>
. -
You don't need to wrap the image in a
<div>
. -
Change
.attribution
to a<footer>
, and use<p>
for the text inside.
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
Remember to specify fallback fonts, in case the font doesn't load on the user's device:
font-family: 'Outfit', system-ui, sans-serif;
. -
On the
body
, changeheight
tomin-height: 100svh
- this way, the content will not get cut off if it grows beneath the viewport. -
Descendant selectors like
.card-container p:nth-child(1)
increase specificity, making the styles harder to override. It's best practice to instead give elements a class, and use that as the selector. -
Remove
width
andmin-height
on the card. -
Add a
max-width
of around20rem
on the card, to prevent it from getting too wide on larger screens. -
I would use
rem
instead ofem
onfont-size
. It's important to never setfont-size
in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
To create the space between the image and the edge of the card, set
padding
on all 4 sides of the card:padding: 16px;
. -
On the image, remove
margin-top
. Adddisplay: block
,height: auto
and changewidth
tomax-width: 100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container.
Marked as helpful1 -
- @MikmameSubmitted 9 days agoP@Islandstone89Posted 9 days ago
HTML:
-
Every webpage needs a
<main>
that wraps all of the content, except for<header>
andfooter>
. This is vital for accessibility, as it helps screen readers identify a page's "main" content. Wrap the card in a<main>
. -
You don't need to wrap the image in a
<div>
. -
The alt text should be written naturally, without using
-
between the words. Write something short and descriptive, without including words like "image" or "photo". Screen readers start announcing images with "image", so an alt text of "image of qr code" would be read like this: "image, image of qr code". The alt text must also say where it leads(the frontendmentor website). A good alt text would be "QR code leading to the Frontend Mentor website." -
Never have text in a
<div>
alone! "Improve your front-end skills by building projects" is a heading. I would make it a<h2>
- a page should only have one<h1>
, reserved for the main heading. As this is a card heading, it would likely not be the main heading on a page with several components. "Scan the QR code to visit Frontend Mentor and take your coding skills to the next level" is a paragraph, so wrap it in a<p>
. Remove the divs, they are not needed. -
Change
.attribution
to a<footer>
, and use<p>
for the text inside. -
Do not use
<br>
to create the space between the<main>
and the<footer>
- this should be done using CSS.
CSS:
-
Make a habit of including a modern CSS Reset at the top of the stylesheet.
-
I recommend adding a bit of
padding
, for example16px
, on thebody
, to ensure the card doesn't touch the edges on small screens. -
To center the card horizontally and vertically, with some space between the
<main>
and the<footer>
, add the following on the body:
justify-content: center; min-height: 100svh; gap: 1rem;
-
Remove
display: block
on the card, as a<div>
is a block element, meaning it takes up the full width by default. -
max-width
on the card should be in rem. Around20rem
works well. -
font-size
must never be in px. This is a big accessibility issue, as it prevents the font size from scaling with the user's default setting in the browser. Use rem instead. -
letter-spacing
must also not be inpx
. You can useem
, where1em
equals the element's font size. -
Since all of the text should be centered, you only need to set
text-align: center
on the body, and remove it elsewhere. The children will inherit the value. -
For the same reason, remove
font-family
on.text
- since you havefont-family
on thebody
, its children inherit the font. -
It's not recommended to set
border-radius
in%
- change it torem
,em
orpx
. -
On the image, add
display: block
,height: auto
and changewidth
tomax-width: 100%
- the max-width prevents it from overflowing its container. Without this, an image would overflow if its intrinsic size is wider than the container.max-width: 100%
makes the image shrink to fit inside its container.
Marked as helpful0 -