Your page looks good @paladinesdragon. Some things to revisit;
- Codepen provides the boilerplate for you. It only expects the code you’d put within the
bodyelement in HTML. (No need to include the
bodytags). For anything you want to add to the
<head>element click on the ‘Settings’ button, then HTML and add it into the ‘Stuff for <head>’ box.
- You can still style the
bodytag in the CSS editor without having it in the HTML editor.
- You can still style the
- Run your HTML code through the W3C validator.
- There an HTML coding error you should be aware of and address. (Using an obsolete attribute)
- Since copy/paste from codepen you can ignore the first warning and first two errors.
- Codepen provides validators for HTML, CSS and JS. Click on the chevron in the upper right of each section and then click on the respective ‘Analyze’ link.
- The one for CSS is good. Use it and address the issue(s).
- (The one for HTML misses things which is why I recommend W3C)
- Check that the user has entered a valid email addr. Throw an HTML5 validation error if not.
- Better because it does not try to submit the form if the field is empty
- Do not use the
<br>element to force line breaks or spacing. That’s what CSS is for.
Cool. Thanks for the feedback.
- Boilerplate in codepen fixed
- Ran code through W3C validator and changed HTML iframe frameborder=0 to use border:none in CSS. Something I will keep in mind when using youtube’s link to get the embed code so thank you.
- Issues in CSS was because I didn’t copy and paste new code from visual studio. Updated and fixed.
- Email address handling is already implemented via JS.
- Removed br element.
Thank you again.
Hey, just a few suggestions:
- I can’t expand the hamburger menu icon with the keyboard. You always want to make sure that you can access all functionality with the keyboard only. The easiest way to solve this is to wrap the
<button>(instead of the
<div>you are currently using) which will automatically make it accessible to a keyboard. Also, you’ll need to have some text associated with that button. You can put text inside the
<button>and visually hide it (my preferred method) or you can use the aria-label attribute to add an accessible name for the button.
- You don’t have an
<h1>heading on the page. It should be near the top of the page. I’m guessing the “Google Pixel 4” heading should be made the
- The two headings “Studio-like photos” and “Without the studio” should probably be combined into one heading.
- You don’t have a
<label>for the email input.
- Move the
<footer>out of the
- Your page has good responsiveness for both changes to view port width and increases of up to 200% in text size. Nice job!
- Hamburger should now be accessible via keyboard and screen readers.
- I intentionally left out the
<h1>tag but it is now added.
- The two
<h2>elements right after another is intentional and as far as I know semantically correct.
- I left out the
<label>element for the email as I didn’t want an empty element. I added aria-label attribute to the input instead.
- Footer is moved.
Thank you for the feedback!
So I’m going to argue that they are not. A heading describes the content that follows it. Two headings at the same level (in this case h2) should have actual content between them otherwise the first h2 isn’t describing anything. If you are making them separate just to get the line break after the first sentence there are other ways to do that.
I checked W3C standards, MDN, accessibility, and other sources prior to making that choice and didn’t find it violated any standard.
Subheadings are an example of this, so I don’t see why or how this is an issue. Whether or not to do it seems subjective. I am however aware there are other ways to accomplish the line break.
Thanks again for your feedback.