Hot answers tagged

41

There are a few good practices that will help this code out. Close the HTML tags that you open - generate valid HTML Newer browsers will probably render your content properly, but older browsers may not. You may also get strange formatting errors that are hard to debug. Save yourself the headache and always close your tags in the reverse order that you open ...


41

Portability / Compatibility: Does it work correctly on all modern browsers (excluding old versions of Internet Explorer)? In Safari, there is a cute tingly-shiny (don't know how to call it) effect as the fully spread pieces fade out. In Chrome, either there is no such effect, or it's so faint it's not visible. (I'm on a Mac.) (Sorry, I don't know enough ...


40

Performance It works fine for me in Opera and Chrome, but it is buggy in Firefox. Profiling reveals that redraw is responsible (big surprise :) ), and there is not much to optimize without changing the whole concept. Two minor optimizations might be: save 2 * Math.PI in a constant. assign context.fillStyle = particle.color; outside the particles loop (...


23

You should always have an Alt attribute in your img tags <img src="http://i.stack.imgur.com/bGZ1m.png" width="80" height="43"> <img src="http://i.stack.imgur.com/bGZ1m.png" width="80" height="43" alt="Toy Airplane - Based on public domain image http://pixabay.com/en/aeroplane-aircraft-airplane-flight-161999/" > new lines added ...


22

Nice idea. I really like what you've done, but some of your implementation can definitely be improved. Let's start with the JSON. Some of those arrays are not needed. You wrap each clothes item in [], there's no need for this as there's just one item in the array! In fact you should only be using arrays to group like items. I'm also going to get rid of your ...


21

This part of the review is focused on the Javascript aspect of the answer I will be really light on this review. I will try to make it short and quick. You have some mixed quotes. Consider the following piece of code: $("#dialog-confirm").dialog({ autoOpen: false, resizable: false, height: 'auto', width: '400px', modal: true, [......


20

From perusing your code a few times: Custom event abuse While it's very cool to have custom events, I would simply remove this extra layer of logic. It does not make sense to call $(networkInfo).trigger('selectNetworkRanges'); if you could just call selectNetworkRanges(). I understand that you would loose access to this but you are accessing networkInfo ...


19

thisButton.attr('data-x') You can use jQuery's .data(). It uses an internal cache for storing key-value pairs, but if it doesn't find it there, it looks up the data-* attributes of the associated element. This means you can do thisButton.data('x'). Note that dash-space becomes camelCase. Number(thisButton.attr('data-x')) If you use jQuery's .data(), it ...


18

That's a big mess of conditionals. Hopefully they all match the comments near them, though given typical code that's likely not the case. You are repeating a lot of HTML code that could be addressed using variables as @V31 demonstrated. If this is the entirety of the code I would probably stop there, but as this system grows you may want to generalize the ...


17

Gah! Thanks for coming here. Essentially, all your if-statements have the following structure: if (scrolled > (($wheresThisAt)+(N * 20))){ $('#salamander-(N + 1)').css('opacity', '1'); So we can comfortably roll that into a loop: for (var i = 0; i < 35; i++) { if (scrolled <= ($wheresThisAt + i * 20)) { break; } $("#...


17

A few things: Variables in JavaScript should be camelCased. PascalCase is traditionally left for types. CSS classes are generally lower-case and hyphenated. Instead of #Name, consider #name (and the corresponding HTML attribute, id='name') Use $ instead of jQuery. $ is syntactic sugar that essentially resolves the same thing, but it's one character instead ...


16

To make it more maintainable and reduce duplication, I'd list the keywords etc. in arrays, and use classes for the styling. That fits nicely as an object with class names as the keys, and an array of strings as values, e.g.: var syntax = { keyword: ["function", "var", "while", ... ], globals: ["null", "this", "undefined"], literal: ["\\d+"] // ...


16

I will only address the repetition in your collision detection code in this answer. The following lines of code are repeated several times: var x1 = $el.offset().left; var y1 = $el.offset().top; var h1 = $el.outerHeight(true); var w1 = $el.outerWidth(true); var b1 = y1 + h1; var r1 = x1 + w1; The only thing that is changed is the value of $el. Therefore, ...


15

Just a few more minor things on top of jt0dd's excellent review. Anonymous functions in JavaScript are awesome, but your program will be more readable if you give the main functions names and declare in advance. That way the main part of your program will become something like this: if (!haveSilverlight) { $('.playbtn').hide(); $('.videoposter')....


14

First of all, lets clean up your code to really leverage the power of .on(): $(function() { var touched = false, previous_touched; function updatePreviousTouched(e){ if(typeof previous_touched !== 'undefined' && previous_touched !== null && !previous_touched.is($(e.target))){ previous_touched.data('...


14

I know I'm late to the game, but each filter function contains essentially the same lines of code. Only the container element differs: function processFilter($container) { $container.find("li.checked").removeClass("checked"); $container.find("li:first").addClass("checked"); $container.find("ul").scrollTop(0); } switch (clicked_id) { case "...


14

Ok first of all.. Silverlight Rant. Silverlight was a flop far less successful than Microsoft expected it to be. HTML5 + WebGL, on the other hand, is the future of handling anything that anyone ever thought Silverlight might do. If you want to maintain highly supported, future-proof practices, start doing some WebGL research. It's INCREDIBLE. Silverlight is ...


14

The first if block can be narrowed by using a color variable; something like this: var color = "red"; // if the person has swiped off but not back on and one hour has elapsed if (Person.DifHours > 1 && (Person.ON === false || Person.OUT !==false)) { color = "red"; // If the person has swiped back on from ...


14

Easily! You can use a regular expression to match page[1-7].html and also search/?: if ($('.content').length && document.referrer.match(/page[1-7]\.html|search\/\?/)) { window.location.href = document.referrer; }; And never write conditions in such wasteful way: if (A && B || A && C || A && D) { when it can be ...


14

Code should read like poetry People who don't know that jQuery(function); is the same as jQuery(document).ready(function); will have a hard time understanding this code. For better readability start your variable names with a $ if it is a jquery object.: var $btn = jQuery(this); Use descriptive variable names. I really hate all those btn, frm, sub, .. ...


14

(I know I'm quite late to celebrate...anyway) I wanted the firework to sparkle, so I created a Color object. When the firework is "old enough", the particles simply begin to sparkle using a random opacity :-) I also addressed @janos remark regarding the end of the animation: the last fireworks now have a couple of seconds to disappear. function ...


14

Use CSS classes rather than inline styles. Instead of modifying the style of each TODO item, use CSS classes. This offloads the styling into a style sheet where it belongs, and it still allows you to test an item to see if it is already completed: function completed() { if ( !$(this).parent().hasClass('completed') ) { $(this).parent().addClass('...


13

The definition of the winningMap is very confusing. I suggest a different form of input: var winningMap = {Rock: "Scissors", Paper: "Rock", Scissors: "Paper"}; It defines which choice can be beaten by a specific selection. If both inputs are the same, it's obvious a tie: var getWinner = function (playerChoice, computerChoice) { if(playerChoice ===...


13

If you're worried about performance, this should raise a red flag: someJQueryObject = $("#some-id"); ... someJQueryObject.html(aPlainOldString); .html(foo) is almost certainly slower than .innerHTML = foo, because .html() does a bunch of extra stuff, like checking if it can use innerHTML, trying to use innerHTML, catching errors if that fails and using a ...


13

First, a word about usage: $().mousePointer is sub-optimal. Why instantiate a jQuery object that you never even touch? Instead, you can attach your plugin as $.mousePointer = ... and call it as $.mousePointer(...). In line 5 you do $.extend({}, {...}, options). The point of passing an empty object as the first argument is to prevent modifying an existing ...


13

You should cache your selector in a variable: var $addedColors = $("#colors_added"); $addedColors.html(parseFloat($addedColors.html()) + 1); Reason: placing selectors in variables is recommended, since overusing selectors can result in poor performance. Everytime you would call a function on $("#colors_added"), the whole DOM has to be parsed by the ...


13

In a sense, your code has already been reviewed, though I disagree with the excessively negative assessment. It is far from being awful and some of the worst JavaScript code ever. you manually concatenate your data string instead of using the JSON library As the documentation for $.ajax() says: The data option can contain either a query string of the ...


12

var seat = $("input[id*=" + seatPrefix + "]"); if (seat != 'undefined') { First off, an extra level of indentation has crept in here. Let me assume this is just a copy/paste error. Any chance you could be using classes here instead of prefixed IDs? Best case: a single ID you know in advance. It seems that you assume there's only one match anyways. ...


12

var top = Math.max(Math.min(Math.ceil((scrolled-$wheresThisAt)/20),35),1); for (var i = 1; i < top; i++) { $('#salamander-' + i).css('opacity', '1'); } This is similar to Amon's answer, but avoids the if structure.


12

I've always been partial to writing JavaScript classes that take a root element: function TabController() { // Maintain reference to "this" in event handlers this.toggleTab = this.toggleTab.bind(this); } TabController.prototype = { $element: null, constructor: TabController, init: function(element) { this.$element = $(element)...


Only top voted, non community-wiki answers of a minimum length are eligible