Random Quote Generator - Feedback

Random Quote Generator - Feedback
0.0 0

#1

It took me a while, but I finally have the quote generator finished… I think. I’m looking for some feedback on it if anyone wants to check it out here: https://codepen.io/jmarkell/full/KQabdw/

Let me know what could be done better and if there’s anything that breaks.

Thanks!


#2

Hello jmarkell,

For some reason I can`t view the random quote generated in the browser. (I am using Google Chrome)
Let me check this on my mobile as well.


#3

Hello jmarkell,

The cause of the above error is because of the below CSS code.

#quote_text {
  position: relative;
  top: -100%;
}

I cant understand why you have given -100% there.This makes the quote to position above the browsers visible space.

To avoid this alignment issue I suggest you nest quote text div inside the quote block as shown below and make #quote_text top value to a positive figure.

Use another div to nest author, so it will not overlap with the quote.

<div id="quote_block">
     <div id="quote_text">
            <q id="quote"></q>
            <span id="author"></span>
      </div>              
 </div>

I suggest you read below documentation about positioning.

I have also noticed that you have saved bunch of quotes in a variable instead of using the $.getJSON and an API to pull data.Try using below API with getJSON.

https://quotesondesign.com/wp-json/posts?filter[orderby]=rand&filter[posts_per_page]=40&callback=

If you need more help feel free to ask.
You have come a long way on this journey.
I am sure with a little bit of research and practice you can be great!.
Cheers! :beers:


#4

Hi Sacheec, its weird that its doing that now. This was working on Chrome, Edge, and mobile last night. In fact it is still working on my mobile.

As to the reason why I set up the quote_text and quote_block that way: I wanted the background to be transparent, but not the text. The only way I found to do that was lay one on top of the other. I wasn’t really happy with that solution - do you know a better way to do it?

I’ll look into how to use that API. Thanks!


#5

Hello jmarkell,

I checked your project now and it seems you have fixed the issue.
Good work!
As for your question, you can make background transparent using below CSS without affecting elements nested inside it.

background: transparent; (With this solution opactity will be 0%)

background-color: rgba(0, 0, 0, 0.3); (Here 0.3 is the opacity and range is between 0 to 1)

Please refer below link to learn more about CSS rgba() function.

https://www.w3schools.com/cssref/func_rgba.asp

One minor thing I have noticed is that you have used inline styling (Styling within html).
Its a best practice to avoid inline styling.Always use CSS to do the styling.
This way your code will be much more cleaner and easy to read and edit.


#6

Yeah, I was able to find that solution yesterday for the transparency. That helped a ton.

I think I have the website set up well enough at this point, however, I was wondering how I could keep the transparent quote block a set height percentage instead of it resizing for each quote length.

Thanks for your help!


#7

I want to give some suggestions for you JavaScript.

#1) There is no need to return false in your tweet function.

#2) There is no need to unbind the click event from the button with id=“tweet-btn”, because there was no click event bound in the first place.

#3) All of the following:

    $("#quote-btn").on("click", function(){
      getQuote();
    });

can be reduced to:

    $("#quote-btn").on("click", getQuote);

and the same can be said for the tweet button click.

#4) You do not need the onload = “getQuote();” on the body element, because you can simply call the getQuote function inside your $(document).ready callback function.

#5) Since you are using the window.open method for accessing the twitter page, you do not an anchor tag in the following.

            <button type="button" id = "tweet-btn" class="btn-style">
              <a href="https://twitter.com/intent/tweet/?text=" target = "_blank">
                <i class="fab fa-twitter fa-lg"> </i>
                </a> Tweet</button>

It can simply be written as:

            <button type="button" id = "tweet-btn" class="btn-style">
              <i class="fab fa-twitter fa-lg"></i> Tweet
            </button>

#8

I did not realize you could set the functions up that way in the $(document).ready.

That cleaned up the code quite a bit.

Thanks a lot!


#9

A post was split to a new topic: Need help with Random Quote Machine project