Hey guy and gals! Anyone want to rip my Product Land Page project apart and tell me how trash it is? Of course, if you have nice things to say, I’m happy with that too.
Your page is not responsive. If I resize the browser to narrow widths, as they would be on a phone, the content should not be cut off, but should reflow so they all fit on the screen. Typically one uses media queries for this purpose; a layout library like Bootstrap or Foundation is another alternative.
Also, the menu buttons need not be so huge, and they really shouldn’t stick to the top of the window, at least not without some background behind them to turn them into a nav bar. Additionally, purple on black is very low contrast, making them hard to read in glaring light conditions, or for people with poor vision. The color you’re using for the border outlines would probably work better for the text – consider swapping them.
Wow. Jerami it is a nice page… I like your nav’s design… AndI know you’re working on responsiveness … Keep up dear…
Hmmm. All 3 media queries work fine for me. My phone version moves to column for me on the 4 phones I have here and when I resize my browser, and when I use dev tools. Maybe it’s your browser. Not really sure as it works from my repo too. https://s.codepen.io/Jerami/debug/JxKvJK/PBkNWNeZpZVM But, thanks for explaining media queries. I get them. They’re at the bottom of the CSS.
I didn’t feel it made sense to block off a whole bar at the top when I could just break out the buttons. As far as contrast, it meets contrast standards for Accessibility but, once again, interesting to hear another opinion. Thanks.
[Edit-update link for debug mode]
I get the same rendering on Firefox, Chrome, and Safari – the menu buttons and images are all cut off, with a horizontal scrollbar appearing. I see two media queries, one for screens larger than 1000px, and one for screens larger than 1600px, but I don’t see any for screens smaller than that. Perhaps your local version hasn’t been pushed to github?
The page was coded “media first”. Didn’t need a media query for small screens. Only screens larger than phones. Didn’t realize anyone was using a desktop screen smaller than 1000px. The github(remote) repo is the one we’re both looking at. It’s the one linked. Shouldn’t be any cut-off, since the page uses box-sizing:border-box. I don’t get a horizontal scroll on any browser or phone. I can’t replicate what you’re getting on any browser or pc or phone that I have here(4phones 3 pc’s). No worries.
But, no worries. Appreciate the input. Have a good day.
Very strange. I can get the effect of your screenshots with the mobile view in the dev tools of chrome, but not firefox’s tools, nor when I resize the browser window. It’s the only page I’ve seen that behaves like that, and I’m baffled myself now. ¯\(ツ)/¯
I bet it’s Ff’s implementation of box-sizing.Probably. Actually, yep. That’s gotta be it.
Perfect. Thanks for letting me know.
EDIT I can replicate the issue with Ff’s Inspector but not on a screen resize in Ff. It may be a limitation of Inspector. Not really sure. ¯\ (ツ) /¯ .
Through all of this, I did notice that my remote was behind on a box-shadow. Thanks for that though! That’s what happens when you try to keep a local a remote and a codepen.
LOL you’re too funny
Ok, all seriousness now, promise.
Your tests come back all passing so that’s good.
So why such dark colors? Just curious.
So, I’m still seeing the responsiveness issues as well, in both codepen and Chrome Inspect =/ I see on codepen Editor Mode that it starts to break at 836px. I see when the responsiveness puts the content into the boxes, which is a nice way to organize everything and keep it from looking like one thing pouring into another.
While your navbar is unique and I like the hover effects of the almost neon looking lights, I think it gets a little muddled when you scroll down the page. It is my own personal preference to have some sort of background to a navbar to make it distinctive enough from everything else on the page, even if it’s transparent and not completely solid. Again, just preference. Keep it any style you’d like.
Your link under your header is sadly a dead link. But if it were to take you somewhere, I’d probably make a button and put the whole text in the button. It will make it stand out a bit more, esp for those who need a screen reader.
I feel that the header “Check out our videos!” and the caption under the video should be centered in the page.
You could probably add the “Join our Mailing Lists for HUGE Discounts!” with the text above it to make it the header for the email box you have. I’d maybe add some margin to the bottom of your newsletter area to give it more balance as well. You could even add a hover effect and design to your Submit button.
Sorry to “rip it apart” lol but I think it’s a good base to make some even better improvements!
“So, I’m still seeing the responsiveness issues as well, in both codepen and Chrome Inspect =/ I see on codepen Editor Mode that it starts to break at 836px. I see when the responsiveness puts the content into the boxes, which is a nice way to organize everything and keep it from looking like one thing pouring into another.”
Yeah, I figured out this is a code pen problem when in editor mode(due to the iframe). It wasn’t coded there so, I didn’t notice until I c/p’d it from VSCode. I wasn’t going to recode it to work on codepen. But, thanks. I did figure it out. You’d have to view the page in debug mode https://s.codepen.io/Jerami/debug/JxKvJK/dXMqBZKVVavr or same code here works fine-> https://jerami76.github.io/fcc-product-landing2/ as far as I can see.
“While your navbar is unique and I like the hover effects of the almost neon looking lights, I think it gets a little muddled when you scroll down the page. It is my own personal preference to have some sort of background to a navbar to make it distinctive enough from everything else on the page, even if it’s transparent and not completely solid. Again, just preference. Keep it any style you’d like.”
Thanks. I was of the opposite opinion that I couldn’t see blocking out a whole bar at the top for 4 buttons. Plus, everyone blocks out the whole bar. I wanted to be unique.
“Your link under your header is sadly a dead link. But if it were to take you somewhere, I’d probably make a button and put the whole text in the button. It will make it stand out a bit more, esp for those who need a screen reader.”
Thanks I missed that. I personally don’t feel buttons are the best thing for internal links, and buttons may not be the best way to emphasize for a screen reader but, once again, I guess that’s personal preference on your part, which is cool.
“Sorry to “rip it apart” lol but I think it’s a good base to make some even better improvements!”
No worries. You didn’t rip it apart. You gave an opinion, which is fine. I’m going to fix the dead link and stick with the other design choices. Appreciate you looking at it and to hear another point of view. The design and color choices were targeted towards dark nightclubs and rocking bands and neon lights. But, I do appreciate hearing what you think.
Honestly it wasn’t the best first impression for me. Granted that web design and graphic design are two separate entities I won’t comment on your style choices. But it’s missing a lot of stuff when it comes to web dev. First and foremost, responsiveness. I saw this page with my phone and everything was just falling off the screen. Normally you design mobile first scaling with mobile users in mind. The benefits of this is that it looks great on mobile and it won’t look too bad on larger screens eitherways. Second is the video. Here it appears like this:
In contrast this is how FCC did it:
Third why is the footer empty?
Lastly padding and margin values are not uniform and in sync with the page. Some places its too much some places its too little.
That’s my dissection! It did pass the test but you can do better
Edit: Got to view it on my laptop. The navigation buttons are even bigger now. So is the footer area. Makes no sense.
Responsiveness, or lack there of, is due to the codepen editor view iframe. To accurately judge a page, you should be viewing in debug mode, lol. Codepen has it’s issues. The page wasn’t written in codepen and it’s silly to code it to work with an online editor when the site is perfectly responsive when not viewed on codepen.https://jerami76.github.io/fcc-product-landing2/
The page was designed mobile first, as you can see by the images that I posted above that aren’t inside a codepen iframe, and you could actually see by just reading the code. Thanks for your input.
I’d be happy to go take a look at it in fact I’m heading over there right now. But the FCC sample landing page has been done well without any of the problems you have mentioned. My tribute page and survey forms also didn’t have any problems I have yet to push my portfolio page to codepen.
Looking at your site now, it still has overflow when you reach a certain point around 840px. The only two differences I see are the black footer is gone on github and there is padding on the
div element that has the id
mailing-list. Other than that it’s very similar. Check if both versions are the same that would be the first thing I would do.
I can see the mobile first idea on both sites but it has some work to it. The elements are all too large on mobile screen size. Possibly what could be casing your overflow issues.
Verdict: both sites are the same.
They are the same code, since I copy/pasted from the github page.
Any chance you could post a screenshot of the breakpoint, not in an iframe?
I can’t replicate it. That’s the problem. Can’t fix what doesn’t appear to be broken, on my end.
before 840px width
after 840px width
I’d love to fix it, if I could see what you’re talking about.
If not, no worries. It passes the tests and I’ve moved well on in the 2 weeks since the OP. Class projects are minor in the grand scheme of things, when the important thing is understanding the code and being able to produce it. It’s a lot like judging a grade 5 art class. Wish I could replicate it on my end to fix it.
Anyway, thanks again for the input.
I don’t think you understood me. I said 840px because everything is fine at 840px. When you reduce the size even further the notification bar starts to leave the page and by the time you reach actual device size it would be pretty bad. I wasn’t talking about above 840px. I would have provided you with a screenshot but I have like more than 20 tabs open my PC would freeze. But that’s that. Nothing to replicate we’re both looking at the same code. Unless or otherwise changed. I was on Google Chrome and Samsung Internet and the results were the same. But you got your tests passing and you’re good I guess.
I would disagree with this though. This is an opportunity to showcase everything that you learned and more. Beyond the certificate, it shows you off. But that’s just my opinion in the grand scheme of things. Good luck with the rest of the projects.
I understand what you’re saying. I don’t think you understand me. I just can’t get the navbar to break under 840px
at 800px vw
or at the extreme of 312px vw
That’s why I was asking for a screenshot.
As far as, this product page being minor, my point is that hopefully, by the time you post an actual portfolio, you’ll have coded many more product pages(or at least 1) that are fully responsive yet not coded to pass a test. I guess if all you’ll have produced at the end of learning to code is the projects done on FCC, then they are very important.
It’s like carpentry, or any other skill where you have to build something. I wouldn’t include the projects that I made in my high school woodworking class as examples of my work, since hopefully by the time I’m showing off a portfolio of my work, I’ll have created better things after I’ve learned rather than projects created while learning. Either way, as I said, can’t fix what I can’t see. No worries. Have a good one.