Quote Machine - Feedback / Critique

Quote Machine - Feedback / Critique
0

#1

Any feedback on the project, but especially on the javascript section would be much appreciated!
I feel like it’s kinda messy and ugly.

Quote Machine


#2

Hi, I don’t think you pen is messy at all. Quite the opposite, I think it’s very clean and minimalistic (which I like). It also isn’t ugly. For what it does, I think it does it well and looks good enough, so why not accept that… Personally, The only thing I don’t like are some of you background colors, I think they are too bright and saturated.


#3

It actually looks pretty good to me. Logical and easy to follow. I would put the click handlers inside the ready function though, just in case: you don’t want people clicking them before the DOM is ready.


#4

I feel like it might look pretty cool if you set a transition (fade out/fade in) when you change the background color when getting a new quote. Looks good overall!


#5

I should have read your question more carefully, I will try again: Regarding your code, I think it looks pretty well structured. Here is what you can improve:

$(document).ready(function() {
  get();
});

Defining a function that only calls another function is redundant. This is better:
$(document).ready(get);

Also, you click handlers contain the line:
e.preventDefault;
This is a function, so you have to call it. Also I wonder whether calling this is even necessary. I think a button that isn’t of type=“submit” does not have a default action, so there’s no need to prevent that.
If I’m right, you can use get as event handler:
$('#another-one').on('click', get);


#6

Hi, I just finished my Random Quote Machine as well. You can have a look at the javascript. I just put it all in one object.


#7

Hey thanks for the reply! I like the saturated colors, but they might be a bit tooo bright.
I appreciate the feedback!

edit: Oh wait, I got confused by your 2 posts because they have different profile pics.

Regarding you second comment:
The e.preventDefault; is redundant, because as you said it has no default action. I removed that!
And this line $('#another-one').on('click', get); works just as good!

Thanks again!


#8

Hey Chad! I placed the click handlers inside the .ready()
Thanks!


#9

Cool Ben! I didn’t know you could do it like that.


#10

Did not know it either, but I learned it a while ago with this free jQuery course.


#11

I think you did a kick ass job mate. well done


#12

This I a very good job. Keep on. Just one thing - the previous button doesn’t navigate to the previous quote or I am just to tired. Anyways I suggest you to have a look at it.
Very nice job.