Random quote generator from API

I have built a random machine generator website. It fetches the quotes from forismatic and has a button to get a new quote. When it gets a new quote, the website color changes. It also features a button to tweet the quote and a button to copy to clipboard (using clipboard.js).

```
//Needs JQuery
function getNewQuote(){
//Changes the text of elements with class .quote-text
//and .quote-author to have the corresponding
//values on the json
var leftQuoteIcon = " ";
//callback function that actually updates the DOM
function updateQuote(json){
$(".quote-text").fadeOut(1000, function() {
$(this).html(leftQuoteIcon+json.quoteText);
}).fadeIn(1000);

var author = json.quoteAuthor;
if (author === ""){//check if author is empty
author= "Unknown";
}
$(".quote-author").fadeOut(1000, function() {
$(this).text("-- "+author);
}).fadeIn(1000);
colorChange();
}

//makes json request
$.getJSON("https://crossorigin.me/http://api.forismatic.com/api/1.0/",
{_: new Date().getTime(), //to prevent caching http://stackoverflow.com/a/31948654/1952996
method: "getQuote",
format: "json",
lang : "en"},
updateQuote);
}

//Open new window with tweet ready with current quote
function tweet(){
var author = $(".quote-author").text();
var quote = $(".quote-text").text();
window.open("https://twitter.com/intent/tweet?text="+quote+" "+author+"&hashtags=FamousQuotes");
}

function colorChange(){
var colors = ["#E74C3C", "#9B59B6", "#3498DB", "#1ABC9C", "#27AE60",
"#F1C40F", "#D35400", "#34495E", "#797D7F"];
var newColor = colors[Math.floor(Math.random()*(colors.length))];
//Change color of body, buttons and text
$("body").css("background-color", newColor);
$(".button-colored").css({"background-color": newColor,
"border-color": newColor});
$(".quote").css("color", newColor);

}

$(document).ready(function(){
getNewQ

From spending some time looking at your code

-
Purely style, but I think grouping statements belonging together and commenting on them sometimes makes your code easier to read

$(document).ready(function(){
  //Take care of the initial quote
  getNewQuote();

  //Take care of click handlers
  $("#newquote").on("click", getNewQuote);//Associate getNewQuote with button
  $("#twitter-share").on("click", tweet);

  //Take care of clipboard
  new Clipboard("#copy-clipboard");
  $("#copy-clipboard").attr("data-clipboard-target", ".quote");
});

-
I would move the magic constants 1000 to a properly named constant

-
A logical OR could make this look much cleaner: ( Returns expr1 if it can be converted to true; otherwise, returns expr2. ) "" and undefined cannot be converted to true.

var author = json.quoteAuthor || "Unknown";
$(".quote-author").fadeOut(1000, function() {
  $(this).text("-- "+author);
}).fadeIn(1000);

• Inside getNewQuote() I would have placed the JSON call on top of the code

-
You can have a list of css classes inside a jQuery call:

$("body").css("background-color", newColor);
$(".button-colored").css({"background-color": newColor,
                          "border-color": newColor});

could be

$("body, .button-colored").css("background-color", newColor);
$(".button-colored").css("border-color", newColor});

Though I do wonder how beautiful changing every single element to 1 color is, I would probably have gone with matching background, foreground, and text colors.

Other than that, this code is readable and maintainable.

StackExchange Code Review Q#136705, answer score: 2