Hi all, if anyone can offer me feedback, I’d be grateful.
I’ve checked color contrast and made the focus and hover states, quite clear. Not 100% sure on the colors for the link. I want something bright, but not identical to the text if possible.
Anyway, looking forward to your suggestions.
Your page looks good @Codemiester.
You don’t have to incorporate if you want but as a suggestion maybe add a little margin/padding on smaller screens so the text is not up against the edge.
Hey, very nice job here. This is one of the few I have reviewed where running some basic accessibility checkers on the page returns 0 issues. Of course automated accessibility checkers are not 100% guaranteed to find all of the issues (you’ll be lucky to find 50% of them) but they do find a lot of the big ones so they are useful.
I do have a few suggestions to enhance accessibility a little, but these are pretty minor so you don’t have to listen to me and they are not intended to be major criticisms.
I would probably not underline the headings. Traditionally, underlines represent hyperlinks, and so that’s probably what they should be kept to. Is anyone going to mistake those for links? Most probably will not since they are also much bigger than the surrounding text. But I think best practices would dictate that you lose the underline on those.
For the content that you have on the page now I would probably lose the current h2 and turn all of the h3’s into h2’s. I’m just not sure that “Starting out” really adds anything since the first heading after that is “Early years” which kind of means the same thing. Or perhaps get rid of “Starting out”, make “Early Years” an h2 and leave the other two as h3’s. I guess I’m saying that if feels like you have one too many headings at the top.
If you are going to keep the underlines on the headings then I would suggest you make the link at the bottom a different color (but leave it underlined). Also, you are increasing the font size when you hover/focus that link and thus it is causing the content to shift. I don’t think this is something most people would find pleasing. I do like the color change and the dashed focus border though.
I’m no expert on fonts but the google font you are using seems to have a very big line height, which by default puts a lot of space between the text lines. It seems you are attempting to undo this by adding a line-height to the body, but this is causing problems with the mouse hover on the link. If I approach the link from below then the hover doesn’t actually take affect until my mouse is almost all they way on top of the link text. I would recommend you take the line-height off of the body so that the font can space itself properly and the hover works correctly. If you don’t like how the lines are spaced using this font then I would not use it as it appears to be created to intentionally have a lot of line height.
Also, while we are on the subject of fonts, setting the font-size to 10px on the body is not having any affect on your page because you are using rem units for all fonts which is computed from the font size of the root element (which would be the html element), not the font size on the body. You could move that to the html element but I don’t think you should. It is better to allow the font size on your page to be determined by the default font size the viewer has set in their browser. Adding a font size in px to the html element overrides their preferences. Besides, if your page is truly responsive then it shouldn’t matter what font size the user is viewing your page with 
Again, great job with this.
Hi @Roma ,
Thanks for your reply. I totally agree with you. I’ve now added a margin at the breaking point and it looks a bit smoother.
Great advice and thanks again.
Hi @bbsmooth ,
Thanks for your reply. Your feedback was excellent. I’ve changed the font to something similar but without the built in line height. I’ve also removed the Starting out header and upgraded the h3’s to h2’s. I also agree about the changing font size for hover and focus and this has been changed to.
Font size… Ah yes, I had got that bit slightly muddled up, I haven’t changed this, but now I know for future that REM is set from the root directory and can be overidden with the :root selector. Thanks for the reminder.
More generally thanks for the fantastic feedback
This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.