Hey all, just seeking feedback on my portfolio page project
Hi,
Good job 
I would change the dark color for the text and also the typography. The header image needs a little more space above it, at least on mobile.
1 Like
Duly noted, How does it look now?
Now you can’t distinguish the section headings. Maybe another color?
This is the way I see it on my phone. You need more space between section on mobile.
1 Like
I can see that. I’ll DL the code and tweak it in a real browser, mobile testing in Codepen is pretty cumbersome. I’ll circle back later, thanks for your feedback!
Ok, I made some changes to this, and the mobile tightness should be gone. I have two immediate questions about this:
-How do people feel about the text-scaling on the portfolio items? Is it distracting?
-Does the navigation feel ok, as if it’s landing in the right place when it moves to a section of the page?
-
I would use transform scale instead of font-size, for your hover style.
-
The reason why the anchor navigation is off, is because of the margin bottom on your portfolio section, it is pushing the section up. Remove it and add margin top to your contact section instead.
-
I don’t think using 100vh on the contact section looks good. Give it a more reasonable height (doesn’t really need a height if you ask me).
-
Your portfolio grid needs some work. Give it a second go, if you need help come back.
1 Like
Agreed, agreed, and agreed. I truncated the text of the portfolio items so they’re of uniform length and none are pushing pictures out of order, and increased the column size when there are just three of them. I think that looks a bit better?
- I gave you wrong information about the link navigation. What you need to do is add top/bottom padding to the projects section. You likely don’t need the margin on the contact section then.
#projects {
padding: 8rem 0;
}
- For the hover style what you need is a default state of transform: scale(1) on all elements that you want the hover style on, and then on the :hover style itself add the change. I would make some classes for the CSS. Also remember to remove the a:hover font-size stuff.
<nav id="navbar">
<li class="nav-item"><a></a></li> <!-- needs to be on the li not the a -->
</nav>
<div class="project-tile">
<a class="project-tile-link">
</a>
</div>
.nav-item {
transform: scale(1);
transition: all .3s ease-out;
}
.nav-item:hover {
transform: scale(1.1);
}
.project-tile-link {
transform: scale(1);
transition: all .3s ease-out;
}
.project-tile-link:hover {
transform: scale(1.05); /* Less scale looks better here */
}
- Try this for the grid. Get rid of the two media queries, make this change to .project-section.
grid-template-columns: repeat(auto-fit, minmax(320px, 1fr));
grid-gap: 4%;
Then unconstrain the images and let the minmax size of the grid control the images.
.project-section img, .home-project-section img {
display: block;
border-radius: 10px;
height: auto;
width: 100%;
}
Then i would take the .contact-links out of the grid selector and give it it’s own settings using flexbox.
.contact-links {
display: flex;
justify-content: space-around;
align-items: center;
}
-
Add a media query to the headings to lower the font size a bit on small screens.
-
Quick note on the usage of h1, you should really just have one h1 and use h2 and h3 for the rest of the sections, apply styles as needed.
1 Like
Where did you learn Grid? I got a lot better after going through Jonas Schmedtmann’s course (this project was written well before then), but you had a lot of code saving suggestions.
There’s one remaining problem with the grid: if there are uneven numbers of projects in sub-sections, the size will be all uneven, like it is now. Though now that I’m typing this, I suppose that can be fixed by only having one project section.
For me your portfolio content is scrolling over your navbar, rather than behind it.
A quick fix would be to whack a z-index:1 on it but the issue is coming from elsewhere and I can’t quite put my finger on it at the moment.
Strangely, if you change the nav to position:sticky it scrolls up out of view before the portfolio section, when it should stay in place once you start to scroll down.
Also in your nav, you have li elements with no parent ul/ol.
From a bunch of different places, Jonas included.
Yes this type of grid is content driven. Otherwise you have to make it a fixed column layout and then control the break points yourself. The upside of it being content driven is…well that it is content driven.
Two column layout:
grid-template-columns: repeat(2, minmax(320px, 40vw));
@media (max-width: 980px) {
grid-template-columns: repeat(1, minmax(320px, 80vw));
}
Funny i actually noticed that but never addressed it, thanks for pointing it out.
It’s because of transform scale. Adding a really high z-index, like 100, to the nav is actually not a bad idea anyway, seeing as it’s fixed and should be on top of everything else no matter what.
Also, as a side note. Apparently you don’t need the initial state of transform: scale(1); although i doubt it hurts to have it, in fact i wouldn’t be surprised if browsers can do some optimizing based on knowing the initial state. Probably in conjunction with specifying the transition property “transition: transform .3s ease-out;”. Specifying the transition property is always better then using “all”.
Yeah, that’s part of this pattern, which I dislike intensely. I refactored this to traditional multi-page application for my portfolio proper, and it looks a lot better. Thanks for all your help with this!
I should go through the Wes Bos Course again. I haven’t seen the Jen Simmons stuff yet, but I probably should.