I’m new here and just started. This tribute page is my very first project and I would appreciate any feedback from experienced campers. Here it is https://codepen.io/dieplumpe/full/yLJQMKZ .
Thank you in advance!
P.S. My choice of colours and forms might be a little boring.
Welcome to the forum!
I think your page looks good.
Just a couple of things.
rel="stylesheet <!--something is missing at the end-->
- Also, for codepen, if you want to include stuff for the head section you can include it in the setting options for the html section.
- Also, you don’t need to include style tags inside the css section
Hope that helps!
Hi Katy, welcome to FCC. I’m just going to touch on one issue, but a super important one. Narrow your browser as skinny as it will go and you’ll see some layout issues with your list. You’ll definitely want to turn that into a single column list at narrower view ports. Since you are using a grid-based layout you’ll want to use a CSS break point to do this. My recommendation would be that you make the default styling on this list a single column (i.e. get rid of
display: grid). Then start with your browser very narrow and slowly widen it until you feel you have enough horizontal space to make two columns. This will be where you put the break point. I recommend you set it using
em units. Using
em units will make it automatically adjust for both changes in width and text size. You can get the
em value by dividing the number of pixels by 16. Then in the break point you can add
display: grid back to the list.
This is a narrow-first approach to styling and I highly recommend you use it for anything you create. Have fun!
Welcome to the forums @dieplumpe. Your page looks good. Some things to revisit;
- Codepen provides the boilerplate for you. It only expects the code you’d put within the
body element in HTML. (No need to include the
body tags). 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.
- The link to the font goes in the box labeled ‘Stuff for <head>’
- Mainly mentioning because you have elements out of order. Everything the browser renders belongs in the
- Run your HTML code through the W3C validator.
- There are HTML coding errors you should be aware of and address.
- Accessibility is about being accessible to all users. Review the lesson about giving meaningful text to links.
- Remove the
style tags from the CSS section. They will cause problems.
style tags are only used for internal styling in HTML. Since all of your styling is external there is no need for them.
- Do not use the
<br> element to force line breaks or spacing. That’s what CSS is for.
Thank you for your feedback. Is it the blue colour for the link necessary? Cause blue doesn’t actually go well with my page’s aesthetic. So is it like a strict rule for all the pages in general?
Your page looks good in Desktop but in Mobile.
Thank you for your thorough feedback!
I put all the things from head section to the Stuff and the title too because the I think it belongs to the head and W3C validator told me the same. And I got rid of body and
I knew about accessibility, I just wasn’t sure how I can implement it. But I tried so please look again. Maybe now when I transfer a link on the “personal information about scientist” it looks more informative.
Yeah, I know. @bbsmooth has already mentioned this issue. And I’m trying to understand how to solve it. But I’m pretty new and it seems like I can’t understand it yet. I’ll keep trying.
hi @dieplumpe, may be Responsive Web Design approach might help in your case
@bbsmooth and @codely I think I figured out how to do this. Please let me know if I did it correctly. But now 2 columns show only on big screens so I think it’s ok.
You can use
@media to change the width in mobile screen.
and for ref you can use
Hope this helps you,
@codely yes, I did it with media. I think you didn’t see my previous message. Thank you anyway!!
If you used
@media then i didn’t work. i have found some articles to make website responsive.
hope this helps,
Happy coding !
@codely But I see it work on my mobile and when I change the size of the display on my laptop. How can I see that it didn’t work? I mean it changes from 2 columns to one when I use mobile or change screen size. What actually doesn’t work?
Sorry, just trying to understand how to see the problem.
Can you detail your question so i can say correctly what are you asking.
Sorry, for asking.
You said that my columns are not responsive to mobile (cause they didn’t get in one column when the size of the screen gets smaller). So I use media and now I see that it responses to changes of the size of teh screen (there is only 1 column on mobile now). But you say it didn’t work. That is why I’m asking what didn’t work and how I can see it. Cause my mobile phone and laptop screen when I change the size of the screen show that I have a response.
So basically I try to understand what you see that I don’t
now it look nice.
There only one left.
Good job cleaning things up, @dieplumpe. I’m going to mention a couple of things;
h1 element is displayed by the browser so it does not go into the
- The tests script is JS. As such, it does not go into the
head. That’s why it no longer shows. Place it back into the HTML section.
I see you’re having some issues with responsiveness. Follow the advice given by @bbsmooth. (Which you did)
But also be aware that when adding margin and padding to widths (left and right) that will affect responsiveness. It’s best to use relative units (em, rem) rather than hardcoding pixel values.
Hi @dieplumpe, congrats, it is much more responsive now. My remaining suggestions on this topic would be to center the
<ul> when it is in a single column and perhaps make the h2 above the list a little more consistent on its width. Currently, just after you go past the break point (going from narrow to wide) the h2 goes from spanning the entire page to being much skinnier than the list. It’s because you have the side margins set to 10em. Personally, I would set them to
auto and set a max-width on the h2 instead.
Speaking of that h2, I’m not sure I would have all of that content in there. Generally headings are much more succinct. I personally would change it to something like “Biography” and then add the extra commentary below it in a
One other accessibility comment. Since you have text in the image you need to put that text in the
alt attribute for the image or put it in the figcaption.
The page looks nice, good job.
I might suggest you give your elements a bit more vertical spacing so everything isn’t as tightly packed vertically.
Just an FYI, the
repeat function should not have a space between the function name and the parentheses.
grid-template-rows: repeat (9, auto);
grid-template-rows: repeat(9, auto);
Fixing it won’t actually do anything visually and you have more than 9 rows. You don’t need to declare the rows so you can remove that property if you want.