My Random-Quote-Machine

Code Feedback
1

Hey all,

I finished my random quote machine and would love some feedback.
I used bootstrap + react.

https://codepen.io/samrosenf/full/vYxeJQo

Thanks!

1 Like
2

Hi @samrosenf !

I think your project looks good!
Congrats!

3

This is real nice @samrosenf. I have a few suggestions:

  • Consider using a <blockquote> for the quote. Take a look at how MDN marks up a block quote.
  • Very nice customization of the keyboard focus indicator on the twitter link and new quote button. On the twitter link you need actual text associated with the link so that screen readers will know how to announce the link. Add the text (something like “Tweet this quote”) to the <a> as normal and visually hide it so it won’t show up on the page.
  • You might consider putting a max-width on the quote box so that the lines of text don’t get too long. Some people have a hard time reading text that is too long. I would recommend you use em units for this.
4

Thanks @jwilkins.oboe!

Thanks @bbsmooth, I have implemented what you suggested.
Since I am using bootstrap I added different column size for the quote box for larger screens. class= "col-sm-8 col-md-7 col-lg-6 col-xl-5"
Which essentially should give the same result as max-width with em units, just for different screen sizes.

Here is the modified version:
https://codepen.io/samrosenf/full/dyvZRvN

5

Looking good so far,

It looks like you took care of a width issue already as someone else suggested. After I checked it out, I found it to feel a bit cluttered, so I inserted a couple of lines

height: 15rem;
margin-top: 5rem;

into your .card-body selector in order to add more emphasis to the card.

The size of the card remains about the same always but the position of the quote inside does change depending on the length of it (how many lines it takes up)

You can fix this by adding this bit

#text p {
position: absolute
}

to your #text p selector.

You would then need to tell your buttons where to go because they would inherit that position, so you would add

.card-body .row .col{
      position: relative;
      top: 7.5em;
}

Disclaimer: While typing this up and fiddling with your pen you were making live changes, so some of this is now irrelevant :slight_smile: Top: 7.5em is way too far down now so you’d need to change it and probably other values as you change.

Keep up the good work and I hope that you have success!

6

Hey @mnichols08 , thanks for the feedback!

I get what you are saying.
Honestly, I thought the whole point of using bootstrap is to avoid fiddling with stuff like that, repositioning everything manually seems to miss the point. I tried what you suggested but than the author stayed in the middle and was on the quote.

I tried a different method for spacing stuff more, I have added some padding to the quote and author, I think it does the job.

This is the result:
https://codepen.io/samrosenf/pen/eYveGwX

That’s what I added:

#text {
  padding-top: 0.8rem;
  padding-right: 0.8rem;
  padding-left: 0.8rem;
  padding-bottom: 0.2rem;
}

#author {
  padding-bottom: 1.8rem;
}
7

Happy to provide my feedback, thank you for sharing!

The code I was suggesting was intended for a previous version of your code so it doesn’t work anymore. You would have to select the figure element instead of the #text p and apply absolute position, then reposition the column containing the buttons.

#quote-box {
  width: 75%;
  position: absolute;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
    }
.card-body {
   height: 20rem;
}

figure {
  position: absolute;
  top: 50%;
  left: 50%;
  transform: translate(-50%, -50%);
}

.card-body .row .col{
      position: relative;
      top: 20em;
}

Applying the above snippet to your existing code as of this post most closely resembles what I was trying to convey in my previous message.

Bootstrap may have classes that help solve this problem as well, but I am not super savvy with Bootstrap.

8

My own quote machine doesn’t do what I am suggesting so please do not take it very serious. Your quote generator is great the way it is!

9

No worries!
By the way, I think it would help next time if besides lines of code, you could describe what is the expected result. I am still not sure what you meant.
But I like what I ended up with, so I am keeping that way :slight_smile:

10

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.