patternphpMajor
Portable website template
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
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
-
You're missing the important viewport and charset meta tags in your head area. Add them before the
-
Watch your ID and class names! You have a pretty loose naming going on in your HTML. A
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
You can use multiple
-
You're sometimes using
-
You using
CSS:
-
I don't know why you're using Arial in your font stack two times, but it's unnecessary. The following is enough:
-
-
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
-
In most cases you can drop the vendor prefixes for the
-
I don't know why you are selecting
Other than this it's generally a bad idea to just select all
-
You're selecting
-
Selectors like
-
Same thing for
-
If you select list-items in a list, don't add
Similar thing for the links:
-
Again, I have no idea what you did there:
Use classes instead of exzessive use of type selectors. Especially if you need it for things like the
-
You have heavy redundancy in your icon CSS. You repeat the
-
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 outlineYou 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 liSimilar 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:hoverContext
StackExchange Code Review Q#41947, answer score: 21
Revisions (0)
No revisions yet.