Random Quote Machine: Please Give Your Feedback

Hello campers,

I completed my random quote machine and is satisfied with it. Please give your valuable feedback and review and let me know if there is anything to change.

Stacks Used: React + Sass + jQuery

Note: It doesn’t have any animation or transition currently. I don’t know how to add one, and the example project freecodecamp gave uses jQuery .animate() function. I don’t understand how to use it. It would be nice if someone suggests any method to implement smooth animations :grinning:


Overall this is pretty nice but there are a few issues:

  • You have aria-hidden set to true on both of the social media links. Was there are particular reason you did that? Setting aria-hidden to true makes it so that screen readers and such will ignore these links. I’m guessing you want everyone to be able to use these links so I don’t think you really meant to do that.
  • You’ve set the width of the quote-box in vw units. This means it will only get wider if I make my browser window wider. If I have to increase the text size to 200% then the width of the box will not automatically adjust as the text size gets bigger and I end up with just one or two words per line in the box (and depending on the browser width, the New Quote button starts to break out of the quote box). The only way to fix this is the make the browser width bigger, but what if I’m not able to? You should probably use a more responsive unit for width, such as em. Personally, I would not set a width on this box but instead a max-width.
  • This is more of a design/style issue but I’m not sure why as I narrow my browser the quote box gets skinnier and skinnier and then suddenly at a break point it gets twice as wide? It just seems a little weird that the quote box wouldn’t take advantage of all that extra horizontal room until you narrow your browser below a certain point. If you go with a max-width instead as mentioned above you solve this issue.
  • Another style issue, the social media buttons switch to a single column just before you reach the narrower break point.
1 Like

Thank you @bbsmooth for your detailed review.

I did not do it. I had set aria-label for the icons. The reason I set aria-label is because screen reader won’t read the icon. The aria-hidden is set by the Babel preprocessor. I don’t know why it is there.

Thanks for noting that. I changed width to max-width and increased the width to 80vw. I removed the media query as well. Now it works nice.

Fixed with max-width.

As I removed the media query, this is also fixed.

Thank you very much @bbsmooth. Your suggestions made my quote machine much better :+1:

This doesn’t make sense to me, but regardless, it needs to be removed. It is an accessibility fail to have aria-hidden set to true on an element that can receive focus.

Otherwise, your other changes are good and this is a pretty rock solid app. Maybe one other suggestion I would make is to change the <section> to <main> as you should have a main. Technically, you should probably have an <h1> as well, especially since you are using an <h2> for the quote text.

Update: I think the reason it is adding aria-hidden=“true” is because you are using font awesome icons. Usually the icon is added using an <i> tag inside of the <a> and aria-hidden would be added to the <i> (which is fine). So add the <i> tag for the icon and then this issue should be solved.

1 Like

Thanks for that tip! It is now solved.

I changed the #quote-box to a main. I will look forward in adding an h1 tag as well :+1:

I’m just going to be a stickler here :slight_smile: The <main> element shouldn’t be inside of a <section>. You will get a validation error on this at https://validator.w3.org/. You don’t need the <section> at all. Just the <main>.

1 Like

Ok. I will change accordingly. :smile:

I like your quote rotator.

The one thing I’d adjust is to set the max-width for #quote-box to just width so that the width doesn’t adjust according to the size of the quote. When the width of the box is jumping around like that, it’s not very comfortable.

Otherwise, nice work!

1 Like

That’s a good point if you don’t want the buttons jumping around horizontally. I would approach this slightly different though. I’d keep a max-width (preferably in em units) and then set the width to 100%. I think you want a max-width because you want to keep the line length of the quote within a reasonable max limit. Setting the width to 100% will always make the quote box as wide as this limit except when you narrow the browser below the max limit, then it will shrink appropriately.

1 Like

@SixStringsCoder thank you for your feedback. But, I had set max-width according to the suggestion of @bbsmooth in previous posts. If I set width, it will be like hardcoding and there will be some responsiveness issues.

I thought %, vw should be used for widths as they will be automatically adjusted. em is probably used for font-size, padding, margin, etc., right?

Also, is there any advantage if I add width to 100% below max-width?

ems are perfectly fine for widths/heights. In fact they are perfect for setting limits on the length of a text lines because they take into account the font size of the text. Personally, I almost always use em units anytime I have to set heights/widths on elements that have text in them. I also use em units for CSS media query break points.

I was able to get the width of your quote box to stay consistent with the following:

#root {
  max-width: 70em;
  width: 100%;
  padding: 0 2em;

70em is just what I thought a reasonable width would be, you might think it is something else. I added the side padding so that as the browser window narrows the quote box doesn’t go all the way to the edge of the view port.

1 Like

Thank you @bbsmooth for the code snippet. It works fine.

No, worries, there are different ways to handle it. If I used width I’d use it with vw or % so that it responds to whatever screen width being used. That said, on small devices another side effect is watching the height jump up and down based on the quote size. Maybe you could deal with that by hard coding a pixel value for height then setting the h2 holding the text to overflow: scroll…maybe, if you like that look.

Another challenge would be to use JavaScript to alter the font-size based on the character count: character counts > 150, lower the font size.

1 Like

Thank you @SixStringsCoder.

I tried to do it, but the code won’t work. Do you have any idea to implement this?

Check the length property.

var [quote, setQuote] = React.useState(random.quote);

So add a console log to see quote.length. If that length is > than a certain amount then you have to change the font size by adding a CSS class that lowers the font-size.

<h2 id="text" className={quote.length > 100 ? "sm-text" : ""}>{quote}</h2>

sm-text is a class you’d set up in your CSS that only sets the font-size to something smaller.

But the other thing is, you may need to change your CSS rules from #text to .text so this sm-text rule can override the .text rule which sets the font-size to 3rem.

1 Like