Tribute Page - Janet Devlin

Code Feedback
1

I’m not 100% happy but what do you think?

https://michaelwright74.github.io/jd-tribute-page/

Hmm why doest the track listing button work?

1 Like
2

So it turns out if you use the FCC Project script outwith of codepen it has to be the last script referenced.

3

Looks good, really like the clean feel and color scheme. Of course, me being me, I can’t really leave it at that. Constructive criticism, here we go.

  • Your container for the content is section id='main', and then within that are a number of sections. Which isn’t really intuitive. Perhaps using <article id='main'>, or even <main id='main'> (yes, main is a perfectly valid HTML5 tag). Seems silly to give it the same id as element, but I think you need that for the tests.

  • Your div id='nav' exists solely to contain a nav element, which in turn contains sub-elements. As a rule, if any block-level element contains a single (usually block-level) element, one of the two don’t really need to be there. Simply remove the wrapping div, and if you need the id, use <nav id='nav'>. Overall, watch for that – if an element contains just one element, one of them can go.

  • Your div id='hero' seems to me to be the article’s main point, yeah? So how about getting rid of the non-semantic div tag, and going with <header id='hero'>...</header>.

  • It’s kind of a thing, but also a little bit of a pain to fix – look into ways to have the sections display below your nav bar. As it is, the h3 in most are being lost.

4

really like the look and how it responds to different devices.
as has been brought up the navigation hides the headings, there needs to be some restructuring on this. It is especially noticeable on the xfactor section.

Great job

5

Hey this is a great first website. The only thing I can add is that when the screen size goes under around 1000px the text is very close to the edge of the screen maybe a small increase in the left and right margin?

Good work though.

6

Thanks snowmonkey, at this stage I really don’t have the whole semantic thing down yet. Need to work on that, so I think that takes up your first 3 points.

As for your 4th point, yes that’s one of the things that is bugging me most, I can’t figure out how to have the h3’s showing when the links are clicked.

7

@ChrisCline1138 - Yup that’s really annoying me tbh, can’t figure out how to sort it.

8

try moving the id’s from the h3 elements to the section itself. It should give you a start at what you’re looking for.

9

@jermur I can only seem to replicate that at 1024x1366 otherwise I don’t want the text going into the hair.

10

here is a cheat I used as I was having the same problem here

(not sure if that is the best way but I’ve only been coding a month so…)
rather than putting margins or padding on the header element I put the link target a few line breaks above my intended target

id="Section_1">
		<br> 
		<br> 
		<br> 
			<header>Section 1</header>`

That fixed it for me and the spacing between sections isn’t all too wonky

11

That works. As I mentioned, moving the id to the section rather than the h3 (or in your case, the header).

Also, rather than using br tags all over, perhaps a margin-top for the header (or h3) would provide the same effect. I get confused, whether to use margin-top or padding-top. Similar but not. :wink:

12

I will have to try that approach

13

@snowmonkey moving the id’s to the section worked for the About and X Factor sections on a xl screen but not on smaller ones and the Discography header was slightly cut off (but that could be a margin-top issue?)

14

Also, how do I get the hamburger menu to close once a link is clicked?

15

I think it is really good! It’s really mobile responsive, the layout is great, and me, in my unprofessional opinion, can’t find anything wrong with it.

16

@LlyogGarmadon Thank you, I’m happy with the responsiveness of it but still not happy with the links.

17

I managed to fix the issue with the anchors by applying the trick from this post (thanks to @tkhquang for that)

1 Like
18

Looks good, and amazing design