Feedback requested on personal portfolio project

Hey everyone, I’m doing the full-stack dev course and hoping to get some feedback on my personal portfolio website. It took me about 50 hours roughly, although I played with features and the design countless times. I am pretty happy with it for now and hope you can let me know if you find any bugs or how it can be improved.

Cheers.

https://leetrw.github.io/

1 Like

Hi, Lee!

Great job on finishing this project. The personal portfolio is a great way to test what you’ve learned and showcase what you’ve accomplished. Overall, this is a simple but great design. The subtle animations and interactions you’ve included work well! But I am a woman who loves white-space. I think my most consistent feedback is to add more space. :sweat_smile: Extra padding, margin, line-height, etc. can create a visual breath of fresh air!

Now I did find some issues with the HTML.

image

  • Here there are <li> nested in a <div>. This is not valid HTML and the <div> should, instead, be a <ul>.

image

  • In HTML, there are elements that are referred to as flow content and phrasing content. <span> is in the phrasing category, and as such, <div> cannot be a child of <span>.

image

  • Here, <div> cannot be a child of <ul>. However, this would be great use of ::after which was used earlier on the <header>

  • The images could use more descriptive alt tags. alt tags should provide clear context and not be too vague.

My last comments on the HTML have to do with the buttons, links, and accessibility. <button> cannot be a child of <a> and vice versa. When deciding between using <button> and <a>, one must think of the purpose it is serving. Does it need to take the user to another page or location (<a>)? Or is it providing a function (<button>)? A great example for button usage would be the arrows for your carousel, though I see they’re not being used that way.

Now let’s consider accessibility. A button is accessible via the spacebar or enter key while links are only accessible via the enter key. It can be confusing for users if one is being used instead of the other. On top of that, for users that navigate with a keyboard instead of mouse, they would need to tab twice to access the link or button that is nested.

For example

Here I have tabbed to the “link” to send an email. When pressing enter, nothing happens. This is because I am currently focused on the <button>. However, if I were using a screen-reader, it would let me know I am focused on a button, so perhaps I would press the spacebar—nothing happens. But when I press tab again…
image

I am finally focused on the link and can access it. As for the carousel “buttons,” they are also not accessible to tabbing. (Though I have a whoooole other opinion on carousels but that’s not important right now, lol) There is a whole workaround to this, but the easiest fix is to just make them buttons.

I lied. Two more accessibility notes. The submit input has no visual cue when tabbed to. This is because the hover is being applied to its container. This is easily fixed by applying the hover (I also encourage you to look into focus) to the input, and again using ::after instead of a <div> for the blue growing effect. Secondly, the descriptions of HTML, JavaScript, and GitHub are also not accessible as they cannot be tabbed to. With everything I have mentioned, you should be able to figure that one out.

I did not have enough time to look too closely at your CSS, but if you would like me to, I can try to come back to it again later to take a peek. :slight_smile:

Great job again! Keep it up!

Thank you for all the feedback, seems I made some rookie mistakes! But I’m still learning.

I have been through and I believe I have fixed all of the issues that you mentioned although I did take some different approaches to what you suggested. Please could you check to see if everything seems in order, it would be greatly appreciated.

Great job applying those tabbing changes! I love it.

One thing I did notice, however, is that you applied a tabindex value to the about me paragraph. I think I understand your logic behind doing so, but users who navigate with a keyboard can use the arrow keys to scroll up and down on a page. So, that information will always be accessible to them. :slight_smile:

Tabbing is important only for elements that have some type of interactivity tied into them.

But again, great job!

Great!

Yes my logic was to make it accessible, apparently I didn’t find the arrow keys whilst playing with the keyboard navigation haha. I have never tried it before now.

1 Like