my second ever site

donalg1

Just put this site together and am interested in getting some feedback, I got the pictures from online and the content I came up with myself just to have something to fill the white space!!

Thanks in advance, here is the link:

http://gordesign.eu

mrmac

Had a quick look, using firefox, no images are being displayed, and css file isn't available.
Can you check that you've loaded the css file and an images folder?

edit: tried it again using IE - and it's fine.

You may need to work on your firefox version.

Gimme a few minutes to have a look at the site, but initially, seems good.

aido179

Couple major issues here.
1. ITS HUGE! One golden rule is never allow your website to scroll sideways.

2. Nothing is aligned properly. Get reading about CSS. I am surprised to see you using divs and an external style sheet, however your style sheet is so messy.
You shouldn't need all those comments to understand what things are doing. There is also a huge amount of redundancy there. i also found a </div> at the start before and <div>s...that's just sloppy.

3. your banner is one huge image. Think about how horrible that must be to load. For example, you don't need the details on the right to be in an image, since they are just text. Cut it down to just the logo, align it to the left and then align the text to the right. (tip: divs are your friend in this situation).

If there was one thing I would say you should do before anything else, it is define a main content div, set its width to be at maximum 900px and align it in the center. The aligning is done in CSS by defining align-left:auto; and align-right:auto;

That will get you on the right track.

[PS. Make a few posts on other threads, even if you just want to say what part of somebody's site you like the look of. ]

donalg1

aido179 wrote: »

Couple major issues here.
1. ITS HUGE! One golden rule is never allow your website to scroll sideways.

2. Nothing is aligned properly. Get reading about CSS. I am surprised to see you using divs and an external style sheet, however your style sheet is so messy.
You shouldn't need all those comments to understand what things are doing. There is also a huge amount of redundancy there. i also found a </div> at the start before and <div>s...that's just sloppy.

3. your banner is one huge image. Think about how horrible that must be to load. For example, you don't need the details on the right to be in an image, since they are just text. Cut it down to just the logo, align it to the left and then align the text to the right. (tip: divs are your friend in this situation).

If there was one thing I would say you should do before anything else, it is define a main content div, set its width to be at maximum 900px and align it in the center. The aligning is done in CSS by defining align-left:auto; and align-right:auto;

That will get you on the right track.

[PS. Make a few posts on other threads, even if you just want to say what part of somebody's site you like the look of. ]

Thanks for looking at it, what were you using to look at it, when I check here on my laptop or desktop I dont need to scroll sideways but I havent checked it on any other ISP's yet

mrmac

donalg1 - not bad for a second site, I've seen worse examples (my own included!!).

aido179 has made some good points:

The width of your page, looks grand on my 15" widescreen laptop, at 1366 x 768 resolution, but you need to be aware that not everyone has this definition.
Restricting your page size to 900px, will help you overcome this.

The banner image: it may be easier to include everything as an image, but that's not the way to do it, as images take way more time to process than text. So, as aido179 suggested, just have the banner use a smaller image and use text for the contact number and email.

Are you a student, or learning this yourself?

Either way, using plenty of comments is to be recommended, especially when you're starting out. It's a handy way of remembering what your css is actually doing, and I often look back at my css files if I'm having a layout problem.

I have a couple of css books, but I really like "CSS Mastery - Advanced Web Standards Solutions" by Andy Budd.

Hope this helps, a bit, and keep tweeking!
I'll get a closer look at it later tonight.

donalg1

mrmac wrote: »

donalg1 - not bad for a second site, I've seen worse examples (my own included!!).

aido179 has made some good points:

The width of your page, looks grand on my 15" widescreen laptop, at 1366 x 768 resolution, but you need to be aware that not everyone has this definition.
Restricting your page size to 900px, will help you overcome this.

The banner image: it may be easier to include everything as an image, but that's not the way to do it, as images take way more time to process than text. So, as aido179 suggested, just have the banner use a smaller image and use text for the contact number and email.

Are you a student, or learning this yourself?

Either way, using plenty of comments is to be recommended, especially when you're starting out. It's a handy way of remembering what your css is actually doing, and I often look back at my css files if I'm having a layout problem.

I have a couple of css books, but I really like "CSS Mastery - Advanced Web Standards Solutions" by Andy Budd.

Hope this helps, a bit, and keep tweeking!
I'll get a closer look at it later tonight.

Thanks for that. I am just learning myself using some online tutorials for about two months now.

I will have a look at the banner and page width issues too I have the banner saved in photoshop so will be easy to fix.

Thanks for taking the time to look at it.

tricky D

The site is broken as you have made similar mistakes to your first one, which you have still not fixed. You were given good advice in the review of your first site yet have not acted on that feedback, so you have not progressed. One of the biggest bugbears for regular reviewers in this forum is people ignoring good advice which is a waste of our time and experience which is freely given.

mahamageehad

First thoughts:

Site doesn't display any css or images in firefox???

Its waaaaaay too wide. The blue background should be in a scalable div so that it readjusts to the browser window.

The logo is awful. The image need to be a vector so that the image is anti-aliased and it doesn't look so messy and bad quality.
As for the rest of the banner, I agree with above. The text should just be text in a div, the only part of that that needs to be an image is the actual logo itself!

I'm not a fan of the orange for the page selected on the nav. On iPhone the navigation looks awful- it splits onto two lines and two orange box covers two different options- home and testimonial.

I think the image on the homepage could be better placed and the text needs padding on the left to push it away from the image a little.

The kitchen page uses two lists: one has a line before each item, one doesn't. Pick a style and stick with it!! On iphone the second list appears over the main body text. The pictures with black borders on the kitchen page are very badly aligned and overlap with the bigger picture.
The wardrobe page has the same padding issue as the homepage.

The testimonials page is awful- the div isn't actually big enough to hold the text and there's a lot of overflow. On iphone the second testimonial appears half way down the page!

The form need a validator and when i pressed submit it says send and then it opens a email in my mail client with the data in it?? Very very bad practise. Either open a mailto: or submit a form, don't mix the two.

It also looks very unprofessional that the images on your gallery pages etc are different sizes! Pick a size and resize them all to that! Some pics in the slideshow (eg 1, 3, 9, 10) are not good enough quality, especially when you make them bigger.

I know it all seems very negative but I am only trying to help! Stay strong!

Deliverance XXV

I can't view it on FF either;

I would *guess* the issues lies in the directory paths and forward/back slashes:
<link rel="stylesheet" type="text/css" href="..\kitchen\style.css">
should be: <link rel="stylesheet" type="text/css" href="../kitchen/style.css">

And...
<img class="banner" src="..\kitchen\banner.jpg" alt="Banner">
should be:
<img class="banner" src="../kitchen/banner.jpg" alt="Banner">

donalg1

Deliverance XXV wrote: »

I can't view it on FF either;

I would *guess* the issues lies in the directory paths and forward/back slashes:
[COLOR="Red"]<link rel="stylesheet" type="text/css" href="..\kitchen\style.css">[/COLOR]
should be: <link rel="stylesheet" type="text/css" href="../kitchen/style.css">

And...
[COLOR="Red"]<img class="banner" src="..\kitchen\banner.jpg" alt="Banner">[/COLOR]
should be:
<img class="banner" src="../kitchen/banner.jpg" alt="Banner">

Nice thanks for that will check it out. I knew the ff version wasn't any good was meaning to check it.

Thanks for the advice.

mahamageehad

Deliverance XXV wrote: »

I can't view it on FF either;

I would *guess* the issues lies in the directory paths and forward/back slashes:
<link rel="stylesheet" type="text/css" href="..\kitchen\style.css">
should be: <link rel="stylesheet" type="text/css" href="../kitchen/style.css">

And...
<img class="banner" src="..\kitchen\banner.jpg" alt="Banner">
should be:
<img class="banner" src="../kitchen/banner.jpg" alt="Banner">

Ah cool i was actually wondering why the fox of fire wouldnt pick it up! :P I had some weird idea in my head that it was because the style sheet was nested and it need to be on the top level, but your way makes waaaay more sense!

donalg1

I have created the following for my CSS stylesheet:

#maxmin {max-width:900px;min-width:650px;margin:0 auto;border:none;padding:0;}

I have added the id to the body element of my html pages, is this the correct way to do this? Or do I use a <div id="maxmin"> above the body and then close it below the body? I haven't updated it on my site yet though just wanted to check it first before doing it.

donalg1

Ok so I have tried fixing the firefox version and managed to slim down the index page however I cant seem to do it in the others even though I am using the same div in the same place, Any Ideas?

Tis driving me crazy!!!

Deliverance XXV

What do you mean slim down the page?
You should try fixing any code errors you have and it may fix any little issues. For example, you have a <div> inserted before the <body> tag which could cause all sorts of cross browser issues.

Deliverance XXV

Just a quick glance over your code and without going deep into your code the main page has a <div class="contwidth"> while other pages don't?

donalg1

Deliverance XXV wrote: »

Just a quick glance over your code and without going deep into your code the main page has a <div class="contwidth"> while other pages don't?

I have just tried that code a friend emailed it to me so put it on one page to see could I get it to work but can't so need to change it tomorrow have given up for tonight!!!

donalg1

Deliverance XXV wrote: »

What do you mean slim down the page?
You should try fixing any code errors you have and it may fix any little issues. For example, you have a <div> inserted before the <body> tag which could cause all sorts of cross browser issues.

Thanks for that

Have fixed that now and a couple of other things, now there are a couple of pages need an overhaul but am getting there anyway.

Giblet

It seems ok, just make sure you don't abuse position rules, especially if you find yourself using large values.
h1.pos_right uses position:relative; right:-555px; when it could simply use text-align:right;

Position can throw off the semantic layout of your page and create inconsistent results.

donalg1

Giblet wrote: »

It seems ok, just make sure you don't abuse position rules, especially if you find yourself using large values.
h1.pos_right uses position:relative; right:-555px; when it could simply use text-align:right;

Position can throw off the semantic layout of your page and create inconsistent results.

Cheers,

I have noticed the position rule makes things a bit messy sometimes alright so was planning on trying to do something else instead of using them. Haven't had a chance to look at it for a couple of days now been putting up Christmas trees instead:rolleyes:

donalg1

Ok I have done some work on the Firefox version and seem to have some of the issues resolved, I still have to get the paragraphs away from the photo's somehow on a couple of the pages, also the Facebook link isnt working I just added it for now.

Just trying to work on the Contact page at the minute to see how to get it to send the email directly without needing to open the senders email provider i.e. Outlook.

I had a 30-day trial with a well known photo editor which has now expired so can't do much with the banner, can anyone recommend a preferably free banner creator if its allowed here?

komodosp

Cool phone number!

pixeldesign

A tip for all beginners:
Before starting webdesign, start learning photoshop or other similar programs like GIMP. Thats how i started.At the moment i draw the design on paper, then i create the graphics, and then code.

mahamageehad

The page is still way too wide.
The background needs to be set to 100% and then all the contact should be within a div that has its left an right margins set to auto, that way the page will adjust to the browser size.

As I said in my first post you need to add padding to the div's where the text is touching the pictures e.g. padding-left:25px; padding-right:25px;

You need a php or javascript script to submit the form to the server without opening the clients web browser.

donalg1

mahamageehad wrote: »

The page is still way too wide.
The background needs to be set to 100% and then all the contact should be within a div that has its left an right margins set to auto, that way the page will adjust to the browser size.

As I said in my first post you need to add padding to the div's where the text is touching the pictures e.g. padding-left:25px; padding-right:25px;

You need a php or javascript script to submit the form to the server without opening the clients web browser.

I tried the padding to get it away from the pictures but couldn't manage it could only get padding to move both but will try it again.

When you say background do you mean the blue or white backgrounds.

donalg1

pixeldesign wrote: »

A tip for all beginners:
Before starting webdesign, start learning photoshop or other similar programs like GIMP. Thats how i started.At the moment i draw the design on paper, then i create the graphics, and then code.

It was photoshop I had originally but that's expired so am looking at gimp now but doesn't seem to be anywhere near the same quality on finished images or logo's

donalg1

aido179 wrote: »

Couple major issues here.
1. ITS HUGE! One golden rule is never allow your website to scroll sideways.

2. Nothing is aligned properly. Get reading about CSS. I am surprised to see you using divs and an external style sheet, however your style sheet is so messy.
You shouldn't need all those comments to understand what things are doing. There is also a huge amount of redundancy there. i also found a </div> at the start before and <div>s...that's just sloppy.

3. your banner is one huge image. Think about how horrible that must be to load. For example, you don't need the details on the right to be in an image, since they are just text. Cut it down to just the logo, align it to the left and then align the text to the right. (tip: divs are your friend in this situation).

If there was one thing I would say you should do before anything else, it is define a main content div, set its width to be at maximum 900px and align it in the center. The aligning is done in CSS by defining align-left:auto; and align-right:auto;

That will get you on the right track.

[PS. Make a few posts on other threads, even if you just want to say what part of somebody's site you like the look of. ]

Tried this and it seems to work fine on FF but then on IE its the full width of the page and looks pretty naff, whereas the way I have it on my site at the minute is working the opposite, looks perfect in IE but then huge in FF

pixeldesign

Ok here are some tips for you:

Open each html file and put everything in the body like this <div id="newdiv"> everything here</div>
The idea is to put :
<img class="banner banhei" src="../kitchen/banner.jpg" alt="Banner">
<br />
and
<div class="para">
in a single div
(see example below)

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>



<title> Kitchen Design, Wexford Ireland - Kitchen, Wardrobe, Furniture Design Ireland</title>
<meta name="keywords" content="kitchen, wardrobe, furniture, interior, domestic, design, consultation, wexford, ireland, modern, contemporary, 
traditional, bespoke, quality, quotation">

<meta name="description" content="Gordon Design specialises in Kitchen, Wardrobe and Furniture design for domestic interiors.  We work with all 
types of materials and can bring your design to life no matter how big or small the task">

<link rel="stylesheet" type="text/css" href="../kitchen/style.css">
<style>
</style>

</head>

<body class="maxmin" style="filter:progid:DXImagetransform.microsoft.gradient(endcolorstr='#000080', startcolorstr='#2447b2', gradienttype='0');">

<div id="newdiv"> <!-- START NEWDIV -->

<img class="banner banhei" src="../kitchen/banner.jpg" alt="Banner">
<br />
<div class="para">
<ul id="navlist1" class="bold"> 
<li> <a href="http://gordesign.eu/index.html"> Home </a> </li>
<li> <a href="http://gordesign.eu/kitchen.html"> Kitchen </a> </li>

<li> <a href="http://gordesign.eu/wardrobes.html"> Wardrobes </a> </li>
<li> <a href="http://gordesign.eu/misc.html"> Other Items </a> </li>
<li> <a href="http://gordesign.eu/gallery.html"> Gallery </a> </li>
<li> <a href="http://gordesign.eu/testim.html"> Testimonial </a> </li>

<li> <a href="http://gordesign.eu/contact.html"> Contact </a> </li>
</ul>
<hr />
<br />

<p style="margin-left:0%;margin-right:4%">
<img style="margin-right:2%" class="imgfll" src="../kitchen/kit3.jpg" alt="Kitchen Picture 3">
  <div style="padding-left:20px"> <span class="bold"> Gordon Design </span> is a <span class="bold"> carpentry </span> firm based in  Co. Wexford and specialises in <span class="bold">

  hand crafted kitchens and interiors. </span>
  We are available to work anywhere throughout the Country and can give a free quotation upon request.
  We provide a very professional service and can meet any requirements a customer may have. <br /> <br />
  There is no job too big or small for us and we offer the <span class="bold"> highest levels of quality </span> possible, we 
  create <span class="bold"> bespoke furniture </span> for any area of the home and aim to please at all times.  <br /> <br />

  To view some of the work we have done previously use the Gallery link above and to read some comments left by previous customers 
  use the <a href="http://gordesign.eu/testim.html"> Testimonial</a> link above. <br /> <br />  Please feel free to <a href="http://gordesign.eu/contact.html"> Contact </a> us at any time to arrange your <span class ="bold">consultation and to get 
  your free quotation. </span> </div><br /> <br /> <br /> <br />

</p>
<br /> <br />
<div class="center"> <hr size="1" />
<p class="footer">  &copy; 2011 Gordon Design	|  <a href="http://gordesign.eu/sitemap.html"> Sitemap </a> |  
<img class="fbook" src="../kitchen/fbook1.jpg"></p>
</div>
</div>

</div><!-- END NEWDIV -->

</body>

</html>

Instead of <body class="maxmin" put <body .
Remove the class because its not the best idea to use them on body tag.
Do the same for all html files.

Now open style.css and do this:

1. Find and remove:
.maxmin {
border: medium none;
margin: auto;
max-width: 900px;
min-width: 650px;
padding: 0;
}


2. Find:
body {
background-color: #000080;
}

and replace with:
body {
background-color: #000080;
margin: 0;
padding: 0;
}

3.Find:
.para {
background-color: #FFFFFF;
border: 3px ridge #FFFFFF;
color: #000000;
font-family: arial;
margin-left: 10%;
margin-right: 10%;
width: 100%;
}

and replace with:
.para {
background-color: #FFFFFF;
border: 3px ridge #FFFFFF;
color: #000000;
font-family: arial;
width: 980px;
}

4.Find:
.banhei {
height: 135px;
margin-left: 10%;
margin-right: 10%;
width: 100%;
}

and replace with:
.banhei {
height: 135px;
}

5.Find:
.banner {
background-color: #EAF5FF;
margin-left: 10%;
margin-right: 10%;
width: 100%;
}

and replace with:
.banner {
background-color: #EAF5FF;
width: 985px;
}


6.At the end of style.css add this:
#newdiv{
margin: 0 auto;
width: 980px;
}


Make a backup before changes and see how it works.I tested it in Firefox, i`m not sure about Internet explorer, but it should work.

donalg1

pixeldesign wrote: »

Ok here are some tips for you:

Open each html file and put everything in the body like this <div id="newdiv"> everything here</div>
The idea is to put :
<img class="banner banhei" src="../kitchen/banner.jpg" alt="Banner">
<br />
and
<div class="para">
in a single div
(see example below)

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN">

<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
<head>



<title> Kitchen Design, Wexford Ireland - Kitchen, Wardrobe, Furniture Design Ireland</title>
<meta name="keywords" content="kitchen, wardrobe, furniture, interior, domestic, design, consultation, wexford, ireland, modern, contemporary, 
traditional, bespoke, quality, quotation">

<meta name="description" content="Gordon Design specialises in Kitchen, Wardrobe and Furniture design for domestic interiors.  We work with all 
types of materials and can bring your design to life no matter how big or small the task">

<link rel="stylesheet" type="text/css" href="../kitchen/style.css">
<style>
</style>

</head>

<body class="maxmin" style="filter:progid:DXImagetransform.microsoft.gradient(endcolorstr='#000080', startcolorstr='#2447b2', gradienttype='0');">

<div id="newdiv"> <!-- START NEWDIV -->

<img class="banner banhei" src="../kitchen/banner.jpg" alt="Banner">
<br />
<div class="para">
<ul id="navlist1" class="bold"> 
<li> <a href="http://gordesign.eu/index.html"> Home </a> </li>
<li> <a href="http://gordesign.eu/kitchen.html"> Kitchen </a> </li>

<li> <a href="http://gordesign.eu/wardrobes.html"> Wardrobes </a> </li>
<li> <a href="http://gordesign.eu/misc.html"> Other Items </a> </li>
<li> <a href="http://gordesign.eu/gallery.html"> Gallery </a> </li>
<li> <a href="http://gordesign.eu/testim.html"> Testimonial </a> </li>

<li> <a href="http://gordesign.eu/contact.html"> Contact </a> </li>
</ul>
<hr />
<br />

<p style="margin-left:0%;margin-right:4%">
<img style="margin-right:2%" class="imgfll" src="../kitchen/kit3.jpg" alt="Kitchen Picture 3">
  <div style="padding-left:20px"> <span class="bold"> Gordon Design </span> is a <span class="bold"> carpentry </span> firm based in  Co. Wexford and specialises in <span class="bold">

  hand crafted kitchens and interiors. </span>
  We are available to work anywhere throughout the Country and can give a free quotation upon request.
  We provide a very professional service and can meet any requirements a customer may have. <br /> <br />
  There is no job too big or small for us and we offer the <span class="bold"> highest levels of quality </span> possible, we 
  create <span class="bold"> bespoke furniture </span> for any area of the home and aim to please at all times.  <br /> <br />

  To view some of the work we have done previously use the Gallery link above and to read some comments left by previous customers 
  use the <a href="http://gordesign.eu/testim.html"> Testimonial</a> link above. <br /> <br />  Please feel free to <a href="http://gordesign.eu/contact.html"> Contact </a> us at any time to arrange your <span class ="bold">consultation and to get 
  your free quotation. </span> </div><br /> <br /> <br /> <br />

</p>
<br /> <br />
<div class="center"> <hr size="1" />
<p class="footer">  © 2011 Gordon Design	|  <a href="http://gordesign.eu/sitemap.html"> Sitemap </a> |  
<img class="fbook" src="../kitchen/fbook1.jpg"></p>
</div>
</div>

</div><!-- END NEWDIV -->

</body>

</html>

Instead of <body class="maxmin" put <body .
Remove the class because its not the best idea to use them on body tag.
Do the same for all html files.

Now open style.css and do this:

1. Find and remove:
.maxmin {
border: medium none;
margin: auto;
max-width: 900px;
min-width: 650px;
padding: 0;
}


2. Find:
body {
background-color: #000080;
}

and replace with:
body {
background-color: #000080;
margin: 0;
padding: 0;
}

3.Find:
.para {
background-color: #FFFFFF;
border: 3px ridge #FFFFFF;
color: #000000;
font-family: arial;
margin-left: 10%;
margin-right: 10%;
width: 100%;
}

and replace with:
.para {
background-color: #FFFFFF;
border: 3px ridge #FFFFFF;
color: #000000;
font-family: arial;
width: 980px;
}

4.Find:
.banhei {
height: 135px;
margin-left: 10%;
margin-right: 10%;
width: 100%;
}

and replace with:
.banhei {
height: 135px;
}

5.Find:
.banner {
background-color: #EAF5FF;
margin-left: 10%;
margin-right: 10%;
width: 100%;
}

and replace with:
.banner {
background-color: #EAF5FF;
width: 985px;
}


6.At the end of style.css add this:
#newdiv{
margin: 0 auto;
width: 980px;
}


Make a backup before changes and see how it works.I tested it in Firefox, i`m not sure about Internet explorer, but it should work.

Cheers for that just what I was looking for you're a legend will check it out tomorrow

donalg1

Ok so I kinda went back to the start on my site and think I have it figured, fixed the size of it and the centering issue, was way too big at first and when I look at it now and how it was I dont know what I was thinking, thanks for the help lads, greatly appreciated. Any thoughts on it now in its current form would be good to hear.

http://www.gordesign.eu

Giblet

Nothing wrong with putting class attribute on a body element, hell you should see my html element!

donalg1

Giblet wrote: »

Nothing wrong with putting class attribute on a body element, hell you should see my html element!

I actually have an attribute in the body element now anyway, and seems to be working fine for me too.

Fokebox

Index page - I think there are too many bold text ... I would take only one style!
Wardrobes page - it differs from other pages I mean the sizes and position of menu ... look like there aren't general style, needs to fix it. The same with "Other items"
The images in the gallery too big ... they upload too slowly .. make them smaller size!
Keep it up and best wishes,
Fokebox

kincsem

^^^ I agree that you have too many bits of bold text in the intro. You are getting close to putting everything in bold, so that nothing stands out.

I like the look of the site. It looks clean and fresh.

The "kitchen" photo seems a bit whited out (if that is a word), and the "other items" photo is a bit out of focus.