Asking for portfolio feedback

Hi, I would like to ask for feedback on my portfolio page, and any projects on it please. Thanks.

Portfolio page.

I'd also like to add that I really enjoyed FCC projects, they were my favourite part of each section.

Since you’ve completed all of these projects I’m assuming you can handle a tougher critique than a newbie might get. Also, I’m assuming you might use some of these projects to showcase for potential employers and thus you’ll want every advantage when competing against other developers, so being especially picky is OK :slight_smile:

First, excellent heading semantics and overall HTML markup on the portfolio page. I usually don’t comment too much on styling since I am not an artist/designer by any stretch of the imagination but I will say that the bold font is a little hard to read at the default size and I find myself cranking up the text size manually in order to read it.

I’m big into accessibility so most of what I point out below will deal with that. Also, I don’t have time to go through every project listed so I’m just doing a spot check here.

  • You’ve got some font size responsiveness issues on the portfolio page. As I increase the font size the nav items at the top start to fall off the left side of the page and there is no horizontal scroll bar to get them back so the only option is to increase the width of the browser. If you don’t know how to manually increase the font size, using Firefox, go to the ‘View->Zoom’ menu and activate ‘Zoom Text Only’. Then while holding down the Ctrl key scroll your middle mouse button to increase the text size. I’m seeing this same issue in your “Pound World” page so I’m going to assume it is an issue in other projects as well.

  • Speaking of the navigation, the Frontend Apps item has a drop down menu on mouse hover but I can’t get the drop down using keyboard only. As a general rule you should make sure this type of functionality works for both mouse and keyboard.

  • On the portfolio page I would recommend you move the images either above or below the text on narrower view ports. And I would highly recommend you use em units for your CSS break points as that will take into account both view port width and text size. I think you’ll also want to set a width (or min-width) on the black image boxes in em so that they are wide enough to contain the text links. For example, as I increase the text size, the “Timestamp App” link text starts to break out of the black box. You can still keep them all the same width, you just want to make sure the width is at least wide enough to contain the longest link text.

  • You’ve got the max-width on your projects-grid set to 900px. Is there a reason you picked this number? It seems a little arbitrary to me. I have a very big monitor and if I need to increase the text size a lot to read the content on your page I can make my browser very wide to compensate. But putting a max at 900px doesn’t allow me to take advantage of that extra width and thus at large text sizes the content feels very cramped with only two or three words on a line. In general you should use units like em to set widths on any elements that have text content so that the content can expand appropriately as the text size increases.

  • Your social media links at the bottom of your portfolio page don’t have any text associated with them, just an <i>. Someone using a screen reader will not know what these links are for because the screen reader will just say ‘link’ or start reading the actual URL for the link because there is no text to read. Add text for each link and you can visually hide it so that screen readers will see it but it won’t actually show on the page.

  • On your portfolio page you are relying on the default keyboard focus indicators that the browser provides and for at least my browser it really isn’t enough (especially with the background color you are using). I always define my own focus outlines and I make them at least 2-3px wide and usually with a dashed/dotted display. There are a ton of different options for this. The main point is that keyboard users should be able to easily see where the keyboard focus is at any time and I don’t think that is the case here. If you don’t like having a strong focus outline for mouse users then there is something you can do to restrict the focus outline to only keyboard users: https://css-tricks.com/the-focus-visible-trick/.

  • Thank you for including alt text for your images but you don’t want to use the word ‘image’ in the alt text (that is implied). A good link for creating alt text: https://webaim.org/techniques/alttext/. Also, since these images don’t really provide any additional content and are more for presentation, I would be inclined to hide them from screen readers completely using aria-hidden="true".

1 Like

Welcome to the forums @alexknz.

I took a look at some of your RWD projects and noticed;

  • None of them pass all the user stories. Keep the test script when forking the pen (<script src="https://cdn.freecodecamp.org/testable-projects-fcc/v1/bundle.js"></script>)
    • The test script should be included, with all tests passing, when you submit your projects.
  • Also noticed a lot of <br> elements in those projects too. It should not be used to force line breaks or spacing. That’s what CSS is for.

I took a quick look at your Markdown Preview project. I have a few of my own markdown pages and pasted some in. On smaller screens I noticed the page wasn’t responsive. I had a single line of code that contained a URL and it fell out of the container on smaller screens.

That’s awesome, thank you. :+1: I will go through each of those points and try to fix it.

I have a couple of questions about using em units in media query break points, I find that concept a bit hard to understand. You mean like this:

@media (max-width: 50em) {...}

Question 1) Where does it inherit the font-size from? <html>?

Question 2) If I want to reduce the font-size in a media query like this:

@media (max-width: 50em) {
  html {
    font-size: 0.8em;
  }
}

At the default font-size of 16px, the break point is at:

50 * 16 = 800px

Let’s say the view port is shrinking towards 800px and when it reaches that value the query runs.

Since the query reduces the font-size, now the break point is at:

50 * 16 * 0.8 = 640px

But the view port is still at 800px, so the query no longer runs?

Yes you are right, these have been modified from the original versions submitted to FCC. I’ve removed the test script and some ids/classes not used for styling.

In the Web Form I have some labels and inputs separated by breaks, like this:

<input type="checkbox" ...>
<label for="...">Checkbox Label</label>
<br>

Do you mean instead I should put it in a block container like a <p>?

For the Markdown Previewer what would be the ideal behaviour? A horizontal scroll bar?

  • Instead of using <br> elements to have each inline element on a new line, use or set container elements to be block-level elements so they’ll each take up the full width.

However you want to handle such that it doesn’t cause a horizontal scrollbar on smaller screens. Here’s the piece of the markdown that I used to see the issue…

## Codepen  
- Codepen creates large and small screen shots of your pens that can be used in your portfolio. Access them from;  
  `https://codepen.io/userName/pen/slug/image/large.png` (for the large screenshot)  
  or  
  `https://codepen.io/userName/pen/slug/image/small.png` (for the small screenshot)
  - where you replace _userName_ with your codepen userId and _slug_ with the id of one of your codepen pens and then copy that link into your portfolio 

Yes, some of them were fun.

When relative units are used in media queries their value is based on the initial value of ‘font-size’ (which I’m assuming is the default font size defined in your browser settings):

So even if you change the font-size after the fact it will not affect the value in the media query.

Also, I would be hesitant to do a global font-size change like that. If you want to scale down some larger headings or something that’s one thing. But user’s should really be in control of how big they need the text to be and doing a global change like that is taking away some of that control. Besides, if your design is responsive to text size changes then there really is no need for it.

As for defining the media query, I use min-width instead of max width because I always use a narrow-first design approach. So I narrow my browser in as skinny as it will go and style the page for that skinny width, which will be my default CSS. Then I gradually widen my browser and add break points when I have enough horizontal room to rearrange, which means I want the break point to happen at a certain point or wider, hence min-width.

2 Likes

Cool, thanks for the clarification. That’s quite helpful.