First page in years, need feedback. Tribute page to Terje Bakken

Code Feedback
1

Hey guys I’m new here and just got to the web design projects section. I think my work on the flexbox and grid were sloppy and probably had some redundant items. Also I noticed the border I applied to #main is not showing up correctly in IE11. Any suggestions? Thanks a lot!

2

It is a simple page, but it does the job fairly well.

  1. The main element is not supported in IE11 (I didn’t even realize this). If you change the main element into a div you can see the border working in IE. But I’d just forget about IE it is pretty much a dead browser.

  2. IE11 only has partial support for CSS Grid, you can check the notes tab on caniuse. Again, I wouldn’t really concern myself about supporting it unless you absolutely have to.

  3. Technically you have set the images container to be a CSS grid but you are using invalid values for the properties.

Invalid:

grid-template-columns: repeat(auto-fit, 1fr);

You can use a fixed size for the repeat or minmax instead.

grid-template-columns: repeat(auto-fit, 280px);

or

grid-template-columns: repeat(auto-fit, minmax(280px, 1fr));

Invalid:

grid-template-rows: repeat(14 2fr);

Here you are missing the comma between the values.

grid-template-rows: repeat(14, 2fr);

Invalid:

align-items: space-between;

align-items does not take space-between as a value, you may be thinking of justify-content.

justify-content: space-between;
  1. I would maybe see if you can uniform the image sizes a bit more. Using object-fit: cover can help if you set a fixed height on the images. However, using images with different dimensions will make it harder to make a good-looking image grid without some extra work and possibly some re-ordering of the images. Something like a masonry layout works well with images of different dimensions but it can be a lot more work to get right.
1 Like
3

Thank you so much for going over it, I really appreciate it. I knew I had mixed something up with the grid but wasn’t sure what. I’m going to go back to work on it with your suggestions.

4

No problem, glad to help.

I know this can be a little overwhelming at first, but I would suggest learning the browser developer tools (Video). The DOM Inspector (Chrome or Firefox) is super helpful. After you have learned the dev tools and used them for a bit you will not be able to go back.

Codepen also has some build-in validators that can be handy to use. If you click the down arrow on the right side of the code boxes you can select the option starting with “Analyze”, it works for all three code boxes.

Happy coding!

1 Like
5

Ok! I think what I have now is much more presentable:

Thank you I’ll take a look at the video and links you shared, I love learning about all this stuff. I’m taking a udemy course as well (Web Dev Bootcamp) and my next subject there is DOM manipulation, but I’ve noticed they skimp over a lot of things there just from doing the exercises at FCC, especially with CSS and basic JS so far.

6

Looks good. I have a few suggestions.

Use classes for common styles. For example, you have this selector:

#image1, #image2, #image3, #image4, #image5, #image6, #image7, #image8, #image9, #image10, #image11, #image12 {
  ...css
}

You should just make a class and give that to all the elements…

.imagegrid-image {
  ...css
}

…or just target the elements using the container as a scope.

.imagegrid > img {
  ...css
}

I’d suggest using grid-gap for the image grid. Give the images width 100% and remove all margin. Now on the grid container set the grid-gap (e.g. grid-gap: 10px).

1 Like
7