I’m looking to get some feedback on my Pomodoro project. I guess there were some copyright issues surrounding the name as its now called 25 + 5 clock. This project took me way longer than I expected and I’ll list some of my roadblocks and concerns here.
- I wrote the project in vs code with create-react-app and moved it over to codepen so that I could have something for people to see and give feedback. There were a lot of hurdles in this alone.
- I struggled with the logic of a timer and how it should work from an accuracy perspective versus a user experience perspective. Despite passing all the tests (*) the timer adds a second every time it switches sessions (the example project also does this) and there are also some potential rounding errors. I tried implementing a more accurate stopwatch but it is jarring from a user perspective and its harder to get it to pass the FCC tests.
- I don’t love the styling of my buttons, and general spacing of things, but this is something I can change later
- In vs code I used css modules and had my components in separate folders each with their own css file. I’m told this is good practice. This was probably more complicated than it needed to be and made it more onerous to move to codepen.
- FontAwesome has a react friendly solution where you install an npm package and import it in order to use fontAwesome icons as react components (e.g.
<MyFaIcon />). Again this didn’t play nice with codepen (the fa components halted the whole page from rendering) and I had to replace the components with normal icon elements.
- My mobile responsiveness could be better
- The test suite fails 1 test because I use two different beep sounds. If I modify it to just use the one it passes.
In general I’m glad to have this project “done” but I’m sure I have some superfluous bits of code, especially the css. I still really feel like I’m throwing stuff at a wall to see what sticks with css.
Hey, just a few suggestions:
- I can’t see the keyboard focus outline when using the keyboard so I don’t know where my current keyboard focus is. It looks like you’ve set the CSS outline property on the buttons to ‘none’. This is bad for accessibility. Change that to something that stands out. I’d recommend you use at least a 2px outline.
- You’ve got too many h1 headings. Ideally, you should just have one at the top of the page. The numeric displays should probably not be headings.
- The buttons don’t have any text associated with them. You might be able to get away with just having the minus/plus sign for the increment/decrement buttons but you definitely need text alternatives for the buttons at the bottom. Right now those buttons just contain icons and a screen reader won’t know how to announce them. You can use visually hidden text for these so only screen readers will see them.
- Ticky-tacky, but I’m getting a horizontal scroll bar just before I reach the break point as I narrow my browser.
- As I manually make the text bigger I am running into some issues. The icons in the buttons are breaking out of their containers. The content in general is breaking out of the page on both sides. I am getting a horizontal scroll bar but I still can’t get the content on the left side of the page back. One suggestion would be to use
em units for height/width instead of px.
- When I shorten the browser the Pomodor Clock header eventually disappears and I can’t get it back with the vertical scroll bar. Also, the black background stops before the bottom of the page.
2 Likes
Thanks for the feedback. I went ahead and made a lot of those changes.
I’m not sure how to fix the very last issue you mentioned though. I did fix the background disappearing by moving the background color to the body, but the title is still disappearing with no scrollbar when you increase the font size enough. My mobile view looks worse now too 
My suggestion is to always start styling your projects with your browser as narrrow as it will go and then work your way wider.
Hey I appreciate the continued feedback. I started from scratch on the styling. Hopefully what I have now is a more mobile first approach. I also removed some of the weird inline styling and consolidated it all to the css file.
I’m still not sure how to handle diminishing vertical sizes or if I should even handle it at all considering is seems like an unlikely edge case. Resizing the width should behave much better though.
It does indeed look better but there are still a few issues remaining:
- Does not handle large increases in text size very well. If you don’t know how to manually increase the text 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. One hint: Use
em units instead of px for width/height on buttons so they can expand with text increases.
- No scroll bars appear (either horizontal or vertical) when I narrow/shorten my browser enough to cut-off content. (Why are you setting
overflow: hidden on the <body>?)
I didn’t realize there was a text only zoom function. I thought that was the same as ctrl + or ctrl - so I learned something there.
Well I had overflow hidden to remove the scrollbars but I take it that’s not a good approach 
I’m seeing now that I still have the problem with part of the app getting cut off if you shrink it small enough or zoom large enough, and even if I remove the overflow hidden rule you can’t scroll to see all the content. I’m not sure how to handle that, is it related to my use of flexbox?
edit
actually looking at the example project it seems to have the same problem as mine, if you shrink it small enough the top of the app gets cut off and you can’t get to it via scrolling.
You’re not alone, a lot of people don’t realize this until it is pointed out. But it is a huge issue for accessibility. As you get older and your eyes start to go you’ll be thankful for it 
Responsive design should be responsive to both changes in view port width and changes in text size. One of the easiest things you can do to ensure both is to use em units instead of px units for your CSS break points. When you increase the text size, the width of the page in em gets smaller (even though the width in px stays the same). Using em units for break points takes this into account. Narrow your browser as skinny as it will go and crank the text size up to 300% and you’ll see that it looks pretty good but the time display is cut-off at both sides. So you’ll want to figure out how to handle that. Maybe show the minutes and seconds on separate lines? Or make it so that you can use the horizontal scroll bar to view it. We generally like to suppress horizontal scroll bars as much as possible but at huge text sizes and narrow view port widths it is sometimes a trade-off we have to make.
Once you have it looking good at skinny/large text, then decrease the text size back toward normal (while still keeping a skinny browser width) and create a break point if needed to as the em width value increases. Then once you have it looking good at skinny/normal text size you can start widening the browser window itself and adding additional break points if needed (again, in em).
One last note, when going from narrow to wide you will add your CSS break points using min-width, not max-width. So your base styling will be based on narrow/huge text, then your first break point might be something like min-width: 15em and then your next one (if needed) might be min-width: 30em. If you do this correctly, then your page is pretty much guaranteed to look good on any display at any width.
1 Like