HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMajor

Portable website template

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
websitetemplateportable

Problem

I have spent the last 6 months as I am studying web development in college to build a website template that is responsive and accessible from as many devices and browsers as possible.

I would really appreciate it if someone reviewed my HTML structure, CSS and share their opinion with me, my aim is to make this site appear nicely on as many devices and browsers as could.

HTML:

```


















Welcome






Header One

Header Two

Header Three

Header Four

Lorem Ipsum is simply dummy text of the printing and typesetting industry dummy text of the printing and typesetting industry Lorem Ipsum is simply dummy text of the printing and typesetting industry dummy text of the printing and typesetting industry.

Paragraph Strong

Paragraph Empaissaied

Paragraph Small
I am a Button


List


List Item

List Item


Sub List Item

Sub List Item




List


List Item

List Item


Sub List Item

Sub List Item




Abber (Hover over me).

Paragraphsubscript.

Paragraphsubscript.

ParagraphMarked Line







Header


Header


Header





Cell


Cell

Solution

HTML:

-
Depending on the language on your website, you should add the lang attribute to your html tag:



-
You're missing the important viewport and charset meta tags in your head area. Add them before the title tag.



-
Watch your ID and class names! You have a pretty loose naming going on in your HTML. A header could appear everywhere on the page. You probably want to be more specific and do something like site-header. Same goes for things like content, page, etc.

This way you will find your self switching between your HTML and CSS files to find out where you actually using the class you currently edit in your styles.

-
I'm not sure if you were just testing the font sizes of your heading elements, but currently you have two h1 elements on your page. There should only be one in the same outline

You can use multiple h1 headings on your page if you create new outlines. This is possible by using HTML5 sectioning elements like article, section, etc. A good resource for this topic is HTML5 Doctor.

-
You're sometimes using br tags after natural block-level elements like h4, ul and p. This is unnecessary.

-
You using div's to create tables. Why? Use table, tr, td, etc. for tabular data. You can use the structure in your html if you actually want to use it for layout. Don't call it a table then.

CSS:

-
I don't know why you're using Arial in your font stack two times, but it's unnecessary. The following is enough:

font-family: Arial, sans-serif;


-
line-height is one of the view properties, where you don't actually need a unit. I had problems with em and % there before, so I use the unit-less value:

line-height: 1.32;


-
Why are you hiding horizontal scrollbars? Unless you have a very good reason doing this, don't.

-
Unless you use a font with special font-weights, you can just use font-weight: bold;.

-
In most cases you can drop the vendor prefixes for the border-radius property. Figure out what you actually need here: http://caniuse.com/border-radius

-
I don't know why you are selecting form div and form div p and why you change the styles for these elements like you did. Adding the bottom margin there should be enough for block-level elements.

Other than this it's generally a bad idea to just select all div's inside a certain element. Assign classes to the div's and use them. The selector form div p could be written as form p.

-
You're selecting input, input[type=checkbox], input[type=radio], textarea, select. Remove input[type=checkbox], input[type=radio], from the selector, because you're already selecting all input elements.

-
Selectors like span.error are overqualified. You should have a better, more descriptive class name and select like .error-invalid-input, etc.

-
Same thing for *.disabled, .... You can shorten this selector by just using the blass there:

.disabled, .disabled:hover


-
If you select list-items in a list, don't add ul to the selector, because a list-item is a child of a list anyway:

#mainMenu .subMenu li


Similar thing for the links:

#mainMenu .subMenu a


-
Again, I have no idea what you did there:

#contactDetails div div {
    display: none;
}


Use classes instead of exzessive use of type selectors. Especially if you need it for things like the display property. You have declared a hidden class anyway, why not use this?

-
You have heavy redundancy in your icon CSS. You repeat the width and height declarations over and over. Use something like this instead:

.icon {
    width: 35px;
    height: 35px;
}

Code Snippets

<html lang="en">
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
font-family: Arial, sans-serif;
line-height: 1.32;
.disabled, .disabled:hover

Context

StackExchange Code Review Q#41947, answer score: 21

Revisions (0)

No revisions yet.