@QuintaFeiraNoite
Submitted
Feedbacks are always welcome :)
@domieee
@QuintaFeiraNoite
Submitted
Feedbacks are always welcome :)
@domieee
Posted
ππ»ββοΈ Hi Davi,
I took a look at your code, and a few little things caught my eye.
Instead of working with values in rem
, use max-width: 300px;
& width 100%;
on your container. then img
get's width: 100%
. I think this would be a bit more elegant.
In this case the text Scan the QR code to visit Frontend Mentor and take your coding skills to the next level
would rather fit into a <p>
element, as the content is not what a heading should contain. Headings should describe something like a page-, oder section content.
After your <main>
element you should rather nest with section
> article
instead of <div>
. Div's are no longer the right way for a good, reachable page
To learn more about web accessibility, check out this link
height: 30.938rem;
on the container should not be necessary. It would be enough to let the content and its paddings and margins control the height.That's what I noticed so, still good job finishing the project!
EDIT: I don't think you need Media Queries for this Project :)
Have a nice day and keep on coding! (βα΅α΄α΅β)
Marked as helpful
@manta-lucian
Submitted
@domieee
Posted
ππ»ββοΈHi manta-lucian,
I noticed only a few small things:
Instead of using div
element, you should rather use a nesting with main
> section
. This is important for web accessibility. Take a look here
Instead of the fixed width of 300px, use max-width: 300px;
+ width: 100%
. The img will keep the width: 100%;
. This way everything remains responsive.
P.S. Change the background-color property of your body π
Edit: Also take a look at the h2
and change it to h1
. h1
has to be the first heading in your code. Here's how you handle headings
Haven't found anything else yet, maybe someone else will find something
Otherwise great project and keep coding!β(α΅α΅α΅)β
Marked as helpful
@edudanntas
Submitted
@domieee
Posted
ππΌ Hi Eduardo,
I have looked at your code and I have noticed some things
For the web accessibility it is important that the h1
element is always at the beginning of the page. A web crawler always looks for the h1
element, and uses its content to describe what the page is about. Also you should nest your code into a <main>
-element
Take a look here
Don't use fixed heights or widths. You can give the container for the image a max-width: 320px;
and a width: 100%
. The img
also gets width: 100%
. So the container adapts to the browser size, and the image to the container. so everything stays responsive.
For the font-size
property you should not specify px
. Instead you can use rem
. px
does not adjust when the brower size is changed.
Read more about rem here
Otherwise it looks great and very similar to the design!
Have a nice day and keep coding!
Marked as helpful
@Abdoulaye33
Submitted
@domieee
Posted
Hi Abdoulaye
I took a look at your code and found a few things you could adjust.
you have given the img
element a height and width of 180px. This would be unresponsive if necessary and would not adjust to its parent element. You could fix this by giving the container a max-width
of the desired width and the width: 100%;
. The img
should also have the property width 100%
.
for web accessibility you should add a <main role="main>"
and nest your code in it. Also you should change your h3
element to an h1
element. this is also important for this case. You can take a look at google and see what is important for web accessibility.
try to use rem
instead of px
for the font size. if necessary the font's wouldn't adjust to the brower size with px.
Keep coding and have a great day
Marked as helpful