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,
[......
11
This review is mostly about improving the implementation.
I don't know JavaScript enough to answer your big questions.
Try to catch @konijn or @flambino in the 2nd monitor chat room.
Naming
The first most noticeable thing to me is the naming of variables.
I think the convention is to use PascalCase for classes,
and camelCase for variables.
JSHint
If you ...
8
Chess clock anyone? :D
HTML:
<!--
Never assume just one. Prepare for more than one always.
With that, we use classes
-->
<div class="stopwatch" data-autostart="false">
<div class="time">
<span class="hours"></span> :
<span class="minutes"></span> :
<span class="seconds"></...
7
(The following is about the HTML only.)
Identation
The identation of your markup is all over the place. I suggest identing by atleast two spaces. I prefer four spaces, though. I’ll append a fixed version below.
Space
Use it.
Separate distinct blocks of code with one or two newlines. Don’t clump elements together like <label>content</label>&...
6
The semicolons after the closing braces are really unnecessary, and the indentation is a bit off here:
if (yDegrees < maxRotation) {
yDegrees = maxRotation;
};
if (yDegrees > 0) {
yDegrees = 0;
};
It would be better this way:
if (yDegrees < maxRotation) {
yDegrees = ...
6
UX (User Experience)
I'll start with a few possible improvements to the User Experience functionality.
In order to offer the best possible interface you have to provide the same or similar experience in all browsers, simply listing something isn't supported, in my mind, is lazy and not a great UX/UI option. I'd look into webshims or a similar polyfill. ...
6
I would suggest loading the external file and initializing the dialog prior to the event...kind of like:
$(document).ready(function(){
// make the dialog
$("#dialog-confirm").load("/help/nameSearch.htm").dialog({
autoOpen: false,
resizable: false,
height: screen.height - 300,
position: {
my: ...
5
First off, don't use the onclick attribute. You have jQuery, so use it to set the appropriate event handler:
$(".accordionHeadingDiv").click(function (event) {
...
});
That will add a click-event handler to all the headings.
Now, as far as I can tell, your code then removes the active class from all other accordion elements, adds it to the one that was ...
5
I've worked for a media company where ads had to run with their own version of JavaScript libraries, in the end we preferred to let ads run in their own iframe. Especially since some ads required access to document.write which creates havoc to your content ;)
It is a bit scary that your solution has to modify jquery-ui.js, I would be very (very) hesitant to ...
5
Formatting
First of all with the excessive whitespace at the beginning of lines,
and the indenting that seems to follow no logic,
this is extremely difficult to read.
For example, this would be so much better:
$('input[name=edit_spitzname]').change(function() {
if ($('#edit_spitzname').val().length > 0 && $('#edit_spitzname').val().length &...
5
Since you're using some ES6+ features, I'd definitely recommend looking into others, such as let, arrow functions, and so on. This website is a good start, and you can find more information by searching "javascript es6".
Adding onto the last point, whenever you don't change a variable, prefer using const for consistency's sake. If you do need to change a ...
4
Some observations:
ResolveJqueryAndLoadAdditionalJsAndCss is too long a function name
ResolveJqueryAndLoadAdditionalJsAndCss lies about what it does
jQuery version '1.10.2' seems awfully restrictive, are you sure ?
You copy pasted the code to load a css file, use a function
console.log in production code is bad
functions in the prototype and variables ...
4
I am guessing that the '.volume' elements are like volume indicators, the louder the sound, the more you show.
You can iterate over the '.volume' elements, compare the index / elementCount to ui.value and decide to hide or show per that comparison.
So, something like :
$('.volumeSlider').slider({
value: 1,
orientation: "vertical",
range: "min"...
4
It'd definitely be nice to get rid of the inlined style attributes. Since you've already ID'ed the sub-menu pretty extensively, it's pretty easy to add this to the styles you already have. E.g.
#privacy-menu .glyphicon {
font-size: 10px;
}
gets rid of the inlined font-size styles.
You can also do the toggling by adding a class to the #privacy-menu, ...
4
Not sure if it is helpful, but since we are on codereview, allow me to give it a shot.
there is a progress element available in HTML5 that works very easily and is fairly well supported (http://caniuse.com/progressmeter), except for in IE off course. I would suggest using this in stead of the (heavy) jquery-ui you use now.
When writing js code, avoid ...
4
With only three different items I don't think there's much benefit from iterating through a list and instantiating the multi-selects automatically. Your version IMO is much more legible than a version iterating through an embedded data structure. Although with lots of selects this could change fast.
You could take the approach of specifying the attributes ...
4
Looks OK to me. jQueryUI will take care of most of the low-level stuff like not letting you open several dialogs and such (by simply not letting you click "behind" the modal).
The code could be cleaned up though:
Store $(item) in a variable (same with the modal element)
No need to quote those option names
Most of the options you're setting are already the ...
4
I believe this is a case of being too paranoid in optimization. Here's some problems:
Your arr is global. You should probably place this somewhere only the widget knows about.
You're clearning the array... by creating another array. You're spawning more objects (in this case, an array) instead of cleaning up. The proper way to clear an array is to set ...
4
There a few simple things you could do to help clean up this code. The first thing I always recommend is to create a closure for your script using an IIFE. This allows your code to run in its own scope and keeps everything out of the global scope. You can also pass in jQuery to the function expression and safely refer to it as $.
(function($, undefined ) {...
4
Instead of destroying and recreating the datepickers, use the (3rd) option method (e.g. .option('maxDate', selectedDate)) to set the minDate/maxDate options when appropriate.
Also, by utilizing the onSelect option, which receives as arguments the selected date and the datepicker instance, instead of using an onChange handler, MomentJS can be eliminated.
...
4
Well, there is a room for improvement in a few areas,
Save references to the elements rather than using them like $("#element_id") every time you need them to avoid DOM Lookup every time
When using .each() you are using $(this) at several places you should save the reference to the element on top of .each() and then use that reference.
You can create a ...
3
You are repeating yourself a ton here, you should read up on DRY.
You could store the animations in an array and then execute those animations :
var PROPERTIES = 0 , OPTIONS = 1;
var animations =
[
[ {top: '425px'} , { duration: 1800, easing : 'linear', queue: true } ],
[ {marginLeft: '-284px'} , { duration: 2500, easing : 'linear', queue: ...
3
Because you have different options for each one, there isn't really an efficient way to combine them into a common selector. And, with only three items, other options aren't really that likely to be more compact. Here's a pure table driven mode that would be more advantageous if you had many more of these:
var multiData = [{
sel: ".choose",
...
3
I would move all positions and other configuration to a simpleObject which contains a condition which is checked to determine if the configuration should be used for the current screen dimensions:
Remove unneeded variables
Added functions to remove duplication
Moved positions to animationConfig so logic and configuration is separated
I made a assumption in ...
3
As Jef Vanzella mentioned, exceptions are for unexpected bad things, not for jumping out of control flow. Hence the second option is better, but it is still substandard code.
Instead of using this, you should have a meaningful parameter name in your function declaration.
Also, you could simply return the falsey !ui instead of !valid.
Finally, not many ...
3
Edit:
Something else that I now noticed while I was reviewing this answer, your if statements could be written differently, it looks like it should be an if/else statement rather than two separate if statements.
We know that if yDegrees is less than maxRotation that it is not equal to zero, nor will it be equal to zero after yDegrees is set to -45(...
3
If you need to externalise/compact the $(".someslider").slider() function, I would do it like this:
External script 'sliders_setup.js', depending on jQuery:
(function(NAMESPACE){ // to keep things clean
NAMESPACE.setup_slider = function(selector, value, selector_result){
$(selector).slider({
animate: true,
...
3
Overall, I would say the code is well written. I can't speak about the jQueryUI parts because I am not as familiar with it. However, there are a couple of things you can do to improve it though. Most of these would be what I would call micro-optimizations.
First, you need to DRY your code as much as possible. For instance, you repeat this line a lot.
...
3
There is too much nesting, which, for me, makes the logic hard to read. A good rule of thumb is to aim for two levels of nesting, by using the Extract Method refactoring.
When the user clicks on the input, nothing happens. When the user starts typing a name that isn't in the list, sometimes the list pops up (if the initial matches) and sometimes they just ...
3
Question responses
What would be your method to improve the management of multiple functions?
Looking at the three functions that call dialog_Handler() it appears that the main redundancies are with the click handlers for the buttons. See the response to the question below for one technique to simplify that logic.
In a comment, you asked:
Do you ...
Only top voted, non community-wiki answers of a minimum length are eligible