So, I've had my Portfolio done for a while but

So, I've had my Portfolio done for a while but
0

#1

I never really got your feedback! :stuck_out_tongue:

https://st3ps.github.io/public/
Do take in consideration that i’ve only gone so far as the front-end bit, no angular, node, react, etc. with that said,
All criticism welcome.

If you’d like to check the source:


#2

Hi, good job, looks really nice! 2 things I noticed: The navigation items are barely readable (on the white background) and the contact section looks strange with everything on the left.


#3

Heeya, thanks you!

You’re definitely right. The first one i’ll get right to.

Repositioning the contact section though…any thoughts how it should be layed out? I could center it all but I wanted to avoid ‘over-centering’…


#4

Alright, so far I’ve changed the top navbar, you should have no trouble reading the menu items on a white background now (do tell if it’s still the case)

I’ve also changed my intro text – I’m not sure wether I should mention “Designer & Front End Web Developer” or just the Web Developer part…
I hear Designers get filtered out, yet just saying Front End Web Development feels lacking…


#5

(1) Do you own the rights to the first background image? If not, you should use an image that you own the rights to.

(2) Related to (1), I’d suggest using a different image anyway, as the current photo is way too gloomy/moody. It might work as a portfolio image for a nature photographer, but an overly moody image like that doesn’t really have a place on a site for a Web developer. You should use an image that conveys either a neutral meaning or a bright/optimistic meaning.

(3) Your scrollspy doesn’t seem to work.

(4) IMO the tiles in your portfolio section are too big. You might consider condensing it down to a 2x3 matrix instead of the 3x2 matrix as it is right now, or alternately reducing the height of the tiles if you want to keep it as a 3x2 matrix.

(5) Code-related: your ‘title’ tag is empty.

(6) Code-related: the contents of your H1, H2, and H3 tag aren’t semantic and don’t use the tags properly. The header tags are headers—you should use them to sectionally divide the body of the entire page, not to arbitrarily visually differentiate lines of text. To style different lines of text, you should use SPAN instead. A more proper use of these tags would be to use H1 for the page title (something like your name), H2 for the “Hi there”, “About Me”, “Portfolio”, and “Get in touch” text, and H3 not at all (as you don’t have any content that calls for another header).

(7) Code-related: it’s improper to use the H1 tag more than once—like an ID attribute, you should have only one H1 on the entire page. You can have multiple H2 tags, but there should be only one H1.

(8) Code-related: you might want to consider reducing your tabs to indent 2 spaces, instead of 4: http://stackoverflow.com/questions/2167524/do-you-indent-your-html-code

(9) You should always run any HTML you write through the W3C Validator to ensure that it’s error-free: https://validator.w3.org/


#6

1, 2) It is a bit artsy, and that’s not ideal for what i’m trying to come across – i’ll have it taken care of.

  1. When I click the elements associated with the scrollspy, they scroll down to the section they’re refering to. That’s what it’s suppossed to do right? If so, it’s working for me…

  2. Reviewing it, you’re right. I’ll have it changed.

5, 6, 7,8, 9) Will work on it, thank you for bringing this to my attention

I’d just like to thank you for reviewing thoroughly my portfolio-- if only I had this done before, perhaps I would’ve had been in more interviews. Who knows.


#7

Nice portfolio! I like the transitions you have on the portfolio items and the fonts you added in.

Suggestions:

  • Have the mobile nav items take up 100% width and be center-aligned.

  • Add a bit more description in the bio area. In addition, you could maybe have someone proofread your bio, to check for errors and get their thoughts.

  • Make it easier to navigate to the portfolio item demos.


#8

Hi, thank you!

  1. Sure, I can try that out
  2. The bio area, as in, the about me part? Well, what else could I say about myself?
  3. Yeah, they are the way they are because of the animations but…usability is taking a hit over it. If you’re on a mobile, it’s definitely not good. I’ll have it restructured soon,

#9
  1. When I click the elements associated with the scrollspy, they scroll
    down to the section they’re refering to. That’s what it’s suppossed to
    do right? If so, it’s working for me…

Actually, taking another look at your code, it seems like you don’t have scrollspy implemented at all. I previously assumed you did because it’s a single-page site and most single-page sites built with Bootstrap tend to use scrollspy, but there’s no evidence of it in your code.

With Scrollspy, basically your navbar will dynamically update as you scroll down the page. I’d recommend that you add it, as it makes your site look more professional.

Bootstrap’s Scrollspy example: http://v4-alpha.getbootstrap.com/components/scrollspy/


#10

:stuck_out_tongue: one more for my to-do list…


#11

Interests, location, fun facts, etc.


#12

So, I’ve got a new version of my portfolio out and…
I
I’ve implemented most of the suggestions around here, some i’m still working around…and a particular few (that i begun through my own volition) are just making me go crazy.

  1. The about section: I can’t for the life of me vertically align the text and I don’t know waht to do anymore.

  2. The w3c validator is telling me i have a closing p tag without a match, but i do have a

    tag, So, I don’t understand … is it because i’vegot an h2 inside the p element, is that a thing you’re not supossed to do?

if you guys wanna check the code throug that or with the browser inspector, i’d just be very grateful


#13

Yup it’s happening because of h2 inside of p. Why didn’t you just put an h2 before the p ?


#14

It is rather odd to have a h2 inside a a p tag. Usually you use a <span> instead for inline editing.

And a simple fix for your vertical about section. Remove the vertical align and add this class:

.row {
  position: relative;
}

.middle {
  position: absolute;
  top: 50%;
  transform: translateY(-50%);
}

Viola! It works!

I love your site, very nice work!


#15

Hi, first of all thank you!

To answer you and nawazishali refering to how i used the h2 tag, honestly i’ve come to a point I don’t really know why I placed it there, but i had been warned about semantics before and it makes total sense, why would you have a header inside a paragraph?
A …uhmn, sleight of mind? :slight_smile:

As for the solution, I don’t know why, i replaced the vcenter class content with the stuff of the middle class you suggested, played around with it and I couldn’t get the result you got…I think it only works from a certain resolution ( try implementing it again and resize the window and see what happens …if it works as intended (text stays centered and doesn’t overflow out of the about section) then I don’t know what I did wrong. I applied it to the same element i was before with the vcenter and yes i changed the row class as suggested.

I figured a different solution.

        .row {
            
            display: flex;
            align-content: center;
            align-items: center;
            
        }

It’ll center things pronto, problem is it completely breaks bootstrap functionality (cols are supossed to occupy full width, and stack on top of each other, which does not happen unfortunately) but i’ll fix that with a media query

Again, thanks guys


#16

Uploaded the almost-final version of this iteration…

So what I know still may need work, as suggested:

Not related to code, but perhaps I do need to add a bit more ‘flair’ to the about section, not only talk more about myself, but refer my services better
mobile experience i imagine it not to be 100% efficient, it’s closer though, need to make small ui changes, some suggested here
in code tab indentaiton could be better i guess, i’ll work on that on the next go.

I’m considering this ok for now, if any of you see this and think there’s something critical i should change i would love to know, as always.


#17

Your new solution is good. I can’t try out the class I gave you, because you changed it, but it has always worked for me on all sizes. I’ll see if I can find the old solution on your GitHub and test it out.


#18

Well, where did you apply the .middle class?
I had a class named .vcenter applied to one of the .cols in the .row, could that’ve been it?


#19

Yeah, I’m looking at your old code right now and playing with it. It should go in the vcenter class. If it seems to work I’ll commit it, and you can merge or look at it if you like!


#20

Nice!

So, wait, i’m kinda new with this whole git thing, if I merge your code, it’ll only change the affected file, right?