41
Semantically, I would suggest using HTML5 elements more.
For example, instead of...
<div id="header">
<div id="logo"></div>
</div>
Use instead: (the ID can stay if you want it to)
<header>
<div id="logo"></div>
</header>
This would change your CSS from #header {} to just header {}
It is common to ...
29
Loading
This is unconventional:
var fizzLoaded;
function fizzLoad() {
if (fizzLoaded) {
return;
}
fizzLoaded = true;
…
}
There isn't any harm in removing the "lock". However, a more elegant way would be to write
window.onload = function fizzLoad() {
…
};
You can use the onload handler to populate the output immediately using the ...
23
One example of where you should use other HTML Elements
<div id="site1Container">
<div id="site1logo"></div>
<div id="site1desc">Site description goes here.</div>
</div>
This should look like this
<div id="site1Container">
<img id="site1logo" src="http://www.google.com/...
21
HTML:
Depending on the language on your website, you should add the lang attribute to your html tag:
<html lang="en">
You're missing the important viewport and charset meta tags in your head area. Add them before the title tag.
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
Watch your ID ...
18
One quick remark: Never leave out the DOCTYPE (<!doctype html> for HTML 5), unless you want to work around the bugs that the browsers will emulate.
EDIT: I forgot the "5" in HTML 5.
Also: It doesn't really matter which DOCTYPE you use, as long it triggers Standards Mode the browser. Browsers don't differentiate between HTML versions. You could use an ...
17
It's pretty good for a first game. There are a few things I would change:
1. Dependencies
You use jQuery for two purposes in your code: $(document).ready to trigger the game setup, and $(document).keydown to catch the keyboard events.
However, there really isn't any need to use jQuery for either of these things, so all it does is add to the load time of ...
15
Names
I find that in your naming, while it appears consistent, the constant use of the fizz prefix is a bit odd, since when you run it you use other words than Fizz or Buzz.
var fizzDiv, fizzFrom, fizzTo, fizzPlayers;
Capitalize first letter
This seems like doubling the amount of work you are doing:
fizzLoaded = true;
var fizzForm = document....
15
About this code:
function TestFibo()
{
InputNum = document.getElementById("InputNum").value;
The convention for function names is camelCase, not PascalCase.
The same for variable names.
Even more importantly,
variables should be declared using the var keyword when used for the first time:
var inputNum = document.getElementById("InputNum")...
14
Well, there's lots of code needing review. I've done my best to split it into each language. I've avoided reviewing Angular, since it's a library I'm not comfortable with.
HTML:
You have tons and tons of ng-* properties.
That is invalid HTML. You must use data-ng-*.
According to the documentation, data-ng-* is valid and will be detected.
Still using <...
14
Tables for layout
This is a clear and obvious misuse of the table element. Tables are for representing tabular data and that's it. Would that same content make sense in a spreadsheet? If the answer is no (as it is in this case), then don't use a table. Use CSS for controlling your layout instead.
Excessive IDs
You don't have a single non-id selector ...
13
Some thoughts on your general code style (some points might depend on personal preference):
I suggest splitting HTML/CSS/JS into different files
Your use of indention and whitespaces is inconsistent
function launchFullscreen(element) {
if(element.requestFullscreen) {
element.requestFullscreen();
has an indention of two spaces
snake.game = (...
13
The idiomatic modern JavaScript way to write a loop over elements of an array is forEach.
The for...in idiom is designed for Object keys, not Array elements,
and it won't work as you might expect.
When using for...in with arrays, it's not the elements that get filled in the loop variable, but their indexes.
For example you can rewrite the loop in fizzing ...
13
There's not much to cover, other than the things @IsmaelMiguel went over, nonetheless:
The Mundane:
This is less of a important thing, as a style thing, but:
font-family: Georgia, Times, "Times New Roman", serif;
font-size: 1.6em;
text-align: center;
background-color: #0033CC;
color: #EEEEEE;
border-top-color: #FFFFFF;
vertical-align: middle;
I find it ...
13
Background: I work as a web developer at a company. I'm the only Angular guy here, and we are using Angular in our greenfield CRM project, so my answer will mainly address Angular (as no one else has here).
id attributes
First thing is first - you're using lots of id attributes. Generally speaking, you only want to use id when it
Is a unique 'object' in ...
12
HTML
You might want to id that nav. It's because it might not be the only nav on the page, and the JS will pick it up and add .fixed-header to it. Something like:
<nav id="main-navigation">
CSS
On the CSS part, if only the nav gets a the .fixed-header class, you might want to consider declaring the CSS for .fixed-header like:
nav.fixed-header{...}
...
11
Going to some HTML5 stuff straight away.
Replace your doctype with the HTML5 doctype. This is necessary for the next few steps
<!DOCTYPE html>
Use the shorter charset meta tag and the meta viewport tag inside your head area. The viewport meta tag is necessary for mobile devices and ensures a good default viewport.
<meta charset="utf-8">
<...
11
The CSS rulesets for your #site1desc and #site2desc etc. are all the same (copy-and-pasted). Instead there could be just one class class="sitedesc" and CSS selector .sitedesc; or several ID selectors for the same ruleset
#site1desc, #site2desc, #site3desc
{
position: absolute;
left:150px;
top: 20%;
}
Your #site1logo and #site2logo etc. ...
11
Several things that would cause me to reject this:
Nearly every element on the page is a <div>.
<div> is semantically void. It says nothing about the structure of the document, other than that "there's a block here". (Except that they're not quite even that, cause you've repurposed a few of them as table cells.)
A huge number of those divs ...
11
Your code is good, but you have one massive oversight:
It's hackable.
Your code shouldn't be accessable from the Console. I would recommend using getters and setters to limit incorrect addition:
var protectedScore = 0; //abstracted away where it cannot publicly be reached
GameClass.__defineSetter__("score", function(scoreToSet){
if (scoreToSet != ...
10
I can't fault your tags at all. The only problem is that due to your table being quite complex, assistive technology may not be able to figure out the structure of your table.
I was going to recommend using scope but according to W3C there are some inconsistency issues.
The row and col values of the scope attribute are currently supported
to a large ...
10
The second option is better, because the possible error is explicit. However, in lots of case in Python, you should follow EAFP and go for the try statement. However, we can do better.
get(value, default)
In BeautifulSoup, attributes behave like dictionaries. This means you can write img_tag.get('class', '') to get the class if it exists, or the empty ...
10
Let's start on a few of the things you explicitly asked questions on before moving over to the code itself.
Are my comments good? I'm coming from Java, and tried to use a Javadoc-like notation, but there doesn't seem to be any way to access the comments besides manually checking the source.
There are a few ways of using comments but most of them stem ...
9
From a once over:
Good
I like how you use an IIFE
I really like how you use direction = keys[event.keyCode];
Not so good
You are not consistently applying the 2nd good technique, for example this:
function setWay(direction)
{
switch(direction)
{
case 'left':
if(old_direction!='right')
{
...
9
for (var i = 0; i < fizzPlayers.length; i++) {
player = fizzPlayers[i];
var val = parseInt(player.value);
if (val != 0) {
active.push(player);
actfact.push(parseInt(player.value));
actname.push(capitaliseFirstLetter(player.id));
}
}
I would change this a little bit
First, I would use the val variable in your call ...
9
Inline-block, float and clear?!
Most display properties are ignored when you float an element (inline-block, table-cell, etc.). Either float it or change it to inline-block, not both. However, what you're really looking for here is this:
label {
display: table;
}
Now you can remove the float and clear properties.
Redundancy
Left is the default ...
9
You should use strict mode in your JS scripts. It will allow you to avoid a lot of common mistakes.
See the section "Changes in strict mode" in the MDN article I linked.
Some of the advantages of using strict mode:
Disallows global variables (when dev "forgets" to declare them);
Disallows duplicate variables (such as declaring ENTER_KEY twice);
Function ...
9
Adding feedback in the form of hints like Select the first position, then Now select the second position would enhance UX.
Now let's nitpick.
Split the huge multi-purpose function into smaller specific ones, group code by purpose
Most of your comments are redundant and just add infonoise to self-explanatory code
Use constants instead of magic values e.g. ...
8
address
I'd enclose the ul.primaryContacts in one address element (not every item separately):
<address>
<ul class="primaryContacts">
<li>Phone: <em class="headerPhone">1.800.corp</em></li>
<li>Email: <em class="headerEmail">office@corpora.com</em></li>
<li>Follow Us: <a ...
Only top voted, non community-wiki answers of a minimum length are eligible
