Would appreciate some feedback,changed content a bit to use some React documentantion,with some repeating data hope thats ok
https://codepen.io/danijel7/pen/qBrBWro
Good Job! I appreciate the larger font and the line height, but the list looks like another paragraph. It’s ok to have bullets on the list in this case or as a quick summary. For most people ,the subtle color on the list is enough to call attention to it, but for me, I can only see it when my eyes are rested or the monitor is at full brightness. At the end of the day I can’t see it.
Okay fixed thanks for feedback
Check this out. list-style-position: inside;
moves the bullet inside the list’s box.
See if you like it.
Now its perfect Good tip I did not like my bullets because of it but did not know I could change position, now I know so yeah thanks
Overall, this is very nicely done. I do have some suggestions though:
-
You have no headings on the page. I would suggest you make “React Documentation” an
<h1>
and then each topic an<h2>
. Technically, you are not required to have headings on your page, but if you do have text on your page that acts/looks like a heading then it must be marked up as a heading. In this case I think the bold blue topics for each section are clearly headings. Just to clarify, I’m not saying change the<header>
to an h1/h2, I’m saying wrap the text with an h1/h2 inside of the<header>
. -
I think the
<header>
in the<nav>
should be pulled out of the<nav>
. This seems like it should be a header for the entire page, not just the nav. Ahh, I see now that having this header inside of the nav is a requirement so I guess you can’t change this. But I still think it is better to have this header as a direct child of the body. -
The light blue text on white background in the nav doesn’t quite meet minimum color contrast requirements. You can use the WebAIM Color Contrast Checker to verify that you have enough contrast. The light blue headings on light gray might get a pass on this because it is slightly bigger text (which would technically only need a 3:1 ratio), but I’m guessing you will want to keep these colors the same and change the heading colors to whatever you make the color in the nav.
-
Kudos for using
em
units for your break points! This gives you “true” responsiveness.
Thank you on clear and detailed explanations and stuff I could improve I really appreciate it . I think its all fixed now And yeah header is project requirement but for future projects one header per page? And its inside body right, thanks again
You can only have one <header>
that is a direct child of <body>
, but you can have additional <header>
s inside of sectioning elements, such as <section>
. So having a <header>
inside of each <section>
is perfectly fine. The one header per page rule applies to the <body>
(only one <header>
may be a direct child of <body>
).
Currently, you don’t have any <header>
as a direct child of <body>
. I was just pointing out that the <header>
inside of the <nav>
should probably be moved out of the <nav>
so it is a direct child of <body>
. But I understand that it is the way it is because of the test requirements.
Okay I understand, hope for some feedback again from you on my portfolio page soon because I will probably have some mistakes there as well, will try not to
Thank you for your time
You are learning, mistakes are expected, they aren’t a bad thing because that’s how you learn. Besides, even professionals make mistakes (pointing the finger at myself). Web development can be quite complicated at times and there is a lot to learn, especially regarding accessibility.
Keep up the good work.
This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.