Skip to content

Instantly share code, notes, and snippets.

@domenic
Created February 17, 2012 23:14
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save domenic/1856049 to your computer and use it in GitHub Desktop.
Save domenic/1856049 to your computer and use it in GitHub Desktop.
Critique of Giordan's page

More or less in decreasing order of importance:

  • rel is misused. Instead of <span rel="x"> it should be <a href="#x">; use <a href="#"> for the undecided ones. You could start using the hashchange event too to make things a bit nicer.
  • <aside> is not used semantically.
  • Put "use strict"; at the top of your JavaScript.
  • That would give you errors if you did something silly, like, say, forgetting a var and thus creating a global variable targ or scrollTo.
  • preventDouble is not used.
  • In fact, your code could use some JSHinting for both correctness and style.
  • The section#decisionList seems to be silly wrapper used only for styling; get rid of it.
  • JavaScript at the bottom of the page (before </body>) for best performance. This would also get rid of the need for the $(document).ready wrapper, but of course you'd still want to use an IIFE to avoid polluting the global scope.
  • type="text/javascript" is not necessary; nor is type="text/css".
  • Any reason for using opacity instead of visibility?
  • It's customary to prefix (or sometimes suffix) cached jQuery variables with $, so $decisionDetails, $targ, etc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment