It would be appreciated if anyone could review the code and critique the design, the code, and the overall feel. I appreciate any help you guys provide. Thanks.
Code: Github
Site: Github Pages
2 Likes
Just a few observations coming from an accessibility perspective.
- I can barely read the headings on your page. You might want to use a different font or make it much brighter.
- I think you made the vertical scroll bars way too hard to use. Yes, those properties are fun to play around with but the width of the scroll bar is only a few pixels wide on my browser, making it very hard to get the mouse exactly where it needs to be, and the color is almost the same as the background so you canât see it.
- The accordion is not implemented correctly. It is not accessible to anyone who uses a keyboard. Here are a few links you can look at for implementing an accessible accordion pattern:
W3C Accordion Pattern
hassel inclusion: My favourite accessible accordion pattern - Removing the vertical scroll from the
bodyand putting it on thesections causes accessibility issues for keyboard users if not done properly. For example, if you are viewing your page with Chrome and you expand the Contact section, you will find that you cannot scroll the content in that section with the keyboard arrows (but you will be able to with Firefox). This is a bug in Chrome (and other browsers) and it doesnât appear that they intend to fix it any time soon. So in order to put vertical scroll on these sections you have to do some other things like addtabIndex="0"to the section and also give it an accessible name (most likely with thearia-labelattribute). I donât expect you to know all of this, Iâm just pointing out that messing with scrolling has accessibility issues and unless you know how to address these it is often better to just leave the scrolling on thebody.
1 Like
Ok, I think Iâve implemented the accordion based on the WCAG pattern you showed me. Is this correct? Also updated the color of the headers for better readability. What do you think?
-Thanks 
Looking for more insight please.
Hello!
I love the single page approach you took. I havenât seen it like this before, very original. To make the look cleaner, Iâd suggest ditch the blue, reduce the color scheme to black and white with the orange/ amber tone as an accent for elements. This will also improve readability of the texts.
UX tip: when I click on the black jack project, I get stuck on the screen with no back options. Best open projects in a new tab or add an extra navigation. Many users wonât even be bothered to use the go back options on their device and will simply leave.
Here is for inspiration one of the best looking single screen sites I have come across recently, maybe you will find ideas for your own:
Your accordion is much better but it still has some issues.
- You canât nest an
h3inside ofbutton, let alone anh2. Get rid of theh3completely, just leave the text. - When I use the space bar on the accordion buttons everything works fine. But when I use the Enter key the accordion section wonât stay closed.
- I can now scroll the About Me and My Projects content with Chrome because you have at least one focusable element in each of those in which I can move the keyboard focus to in order to scroll. If you can add a focusable element to the Contact Me section then you will be good for all three sections.
- You are correctly setting
aria-hidden="true"on the closed sections, but you must also remove them from the DOM as well. The easiest way to do this is to use CSSdisplay: nonebut you can also use thevisibilityproperty (as you are doing with the project boxes in the My Projects section).
In addition to the accordion, there are a few other issues:
- Do not add click events to non-interactive elements such as
spans. If it is a link then use anaelement. - One test that your page needs to pass in order to be considered accessible is to expand the browser to fill up the entire screen and then use the browserâs zoom feature to zoom in 400%. At that zoom level you should still be able to read all of the content and access all of the functionality. If you do this I think you will agree that it is very hard to access the content. I would mark this as a fail if I were doing an accessibility audit. You may want to consider changing from an accordion layout to a tabs pattern when space is limited. You could probably also condense the header so it doesnât take up so much vertical space on the page.
Sorry if I sound like a broken record here but accessibility is an important issue and it happens to be my specialty. Imagine if you were to use this to apply for a job and the person reviewing your portfolio was hard of seeing/blind or could only use a keyboard. If your page isnât accessible to them then you probably arenât going to get an interview.
1 Like
Thank you for the feedback. I will fix the UX issues while working on the readability.
add, to the project link functionality, so the projects opened in new tabs.
If projects open in same tab, folks need to do additional moves to get back to your portfolio.
Also, maybe add links to projectâs repos
You do not sound like a broken record. Having full accessibility is very important, and I do appreciate your input. You have given me some great points to look into, and these will undoubtedly improve the usability of my site and seem like best practices in general.
Thanks
1 Like
Would a media query at, say, max-height: 350px satisfy this use case? I could switch the layout to scroll on the body instead of the sections, allowing the full section to expand and be visible. While also setting styles to reduce the header space taken up.
1 Like
Hi,
First I love the first screen on the design!
I would say that on the projects section, it would be better to add a link that takes me to the actual project, you have this implemented when the user clicks on the image, which I first thought it was to make it bigger.
I would also separate the name of the project and the title that says âtechnologies usedâ, since I think the user can find this confusing to known the actual name.
About the code, I think it would be a good idea to separate the files that are getting to large, for example the index.html, is growing fast, and itâll be harder to work on that.
Hope it helps and keep it up.
1 Like
Yes, I think you are on the right track with a media query targeting the height. Instead of using px units you might consider using rem instead, which will also take into account if the user has manually increased the font size on the page.
Keep up the good work. Learning about accessibility will only make you more valuable to a potential employer. And I personally find it a fun challenge trying to make things both interesting and accessible.
I agree, the challenge and the learning process is the best part. Wouldnât of choose to pursue web development otherwise.
So, I think Iâve found a solution to the height issue.
CSS media query targeting max-height at 20rem, which should work with a window height of around 320px and below. I also expanded all the sections to their max height using JavaScript and the window.matchMedia function at the max-height: 20rem. An event listener on the window object for âresizeâ switches between the layouts and an initial check if itâs zoomed on the initial page load. If it is zoomed initially, it automatically uses the zoomed layout; otherwise, it uses the normal one.
This only works if the page is zoomed. If the window is resized and has a width above the 768px breakpoint, the header is still verticle on the side and looks odd. I may have to add another break somewhere, but Iâm not entirely sure how that would look. Maybe ((max-height: 20rem) and (min-width: 768)) or something???
The new code is on my GitHub, if you could check it out, it would be much appreciated. 
Thanks again for all the help.
Nope better solution for the header above the 768px breakpoint⌠A simple position: sticky; top: 0, and height 100vh. Works like a charm. 
Thank you for sharing that accordion build.
Good luck on your guild.
1 Like