Random Quote Machine Feedback 🙏

[robaish]

Hey campers,

Submitted the random quote machine challenge using React and SCSS. Would love some feedback if you have the time Thanks and happy coding!

The Great Quotes (FCC)

[bbsmooth]

Overall a nice job on this. I do have a few suggestions. Most of these have to do with accessibility. I don’t expect you to know all of this stuff or even implement all of it. My main goal here is just to make you aware that these issues exist and that accessibility is becoming increasingly important.

• I would not put a height on the quote-box div. You generally want an element to grow as tall as needed and putting a height on this div prevents that. When I manually increase the font size the buttons and content eventually break out of the div because it can’t grow taller as needed.

• I like that you put a max-width on the quote-box div instead of just a width but I would recommend you use rem units instead of px for the same reason as above, it will allow the div to widen to accommodate larger text sizes.

• The absolute positioning of the quote-box div is also going to cause it to overlap the heading when the text size is increased. I would suggest that a much more robust method for positioning the three children of the wrapper div is flexbox (or grid if you prefer).

• I think a <blockquote> is probably the best element to use here. Check out how MDN implements a block quote.

• I can see the keyboard focus indicator on the twitter link but I can’t see it at all on the new quote button. Rather than rely on the browser’s default focus indicator I always suggest you customize the CSS outline property to make sure it is clearly visible.

• Every <img> tag needs an alt attribute. Concerning the twitter icon image next to the link, in this case I think the alt text should be empty (alt="") since it is not actually part of the link and the link has text associated with it. The reason you want empty alt text here is to prevent screen readers from announcing the image (basically hiding it from the user) since it is purely decorative.

• Technically, all of the content should be wrapped in a <main> tag. I’d just change the wrapper div to main.

• Generally, when you see a div at the bottom with something like class="footer" on it then it is a pretty good hint that the div should be changed to a <footer> and should probably be placed outside of the <main>.

• Any time a link/button opens a new window you want to indicate to the user that this will happen. (In this case I’m just talking about the link to your github at the bottom since the twitter link opens in the same window). You need to do this both visually and with text (for screen reader users). Visually, there are a number of standard icons that you can place directly after the link text (just google “new window icons”). For text, one way would be to simply add " (opens in new window)" to the end of the link text and then visually hide it.

[robaish]

Thanks for the thoughtful pointers, very much appreciated!

I need to get a better grasp of accessibility, and I’m often in trouble with responsive design when it comes to height – especially resized windows causing weird aspect ratios. Sometimes I’ve declared all kinds of rules for varying aspect ratios, but I feel like there has to be a simpler way.

For the first point about height on the quote-box div, any suggestions for making sure the box always keeps the same size regardless of quote length? I don’t want it to grow and reduce in size, so simply declared the fixed height.

[bbsmooth]

@robaish

Sometimes I’ve declared all kinds of rules for varying aspect ratios, but I feel like there has to be a simpler way.

I think you need to let your obsession with aspect ratios go The very nature of the web makes it very hard to keep the layout pixel perfect because you want the page to be flexible enough to fit on any type of device the user can throw at you. The primary concern should be that the content displays in a manner that is readable.

@robaish

any suggestions for making sure the box always keeps the same size regardless of quote length? I don’t want it to grow and reduce in size, so simply declared the fixed height.

In order to do this you would have to know in advance what the longest quote is and then make the height of the box tall enough to fit that longest quote. If you don’t know that then I suppose you could just way overestimate. But if you do put a height on that div you should use rem units not px, this way the height will grow proportionally to the font size (which is a good thing).

[robaish]

@bbsmooth

I think you need to let your obsession with aspect ratios go The very nature of the web makes it very hard to keep the layout pixel perfect because you want the page to be flexible enough to fit on any type of device the user can throw at you. The primary concern should be that the content displays in a manner that is readable.

Food for thought

@bbsmooth

if you do put a height on that div you should use rem units not px , this way the height will grow proportionally to the font size (which is a good thing).

Ah gotcha, you mentioned the rems already before and that’s the solution. Thanks!