Trying to describe a document a bit more clearly..

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
Charles256
DevNet Resident
Posts: 1375
Joined: Fri Sep 16, 2005 9:06 pm

Trying to describe a document a bit more clearly..

Post by Charles256 »

Code: Select all

<div id='login'>

			<fieldset>
				<legend>
					User Login
				</legend>
				<div class='form_entity'>
					<span class='form_explanation'>
						Username
					</span>
					<span class='form_input'>
						<input type='text' class='text_button' name='username' />

					</span>
				</div>
				<div class='form_entity'>
					<span class='form_explanation'>
						Password
					</span>
					<span class='form_input'>
						<input type='password' class='text_button' name='password' />
					</span>

				</div>
				<div class='form_entity'>
					<span class='form_submission'>
						<input type='submit' class='submit_button' value='Login' />
					</span>
				</div>
			</fieldset>
		</div>
Can anything think of anything else that I could possibly define? Are any of the names unclear? I'm doing it like this so later on another user can come in and change any part of the form rather easily. Or such is my hope. All input appreciated.
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

Use the <label> tag, brilliant tag so many people don't use, get rid of form_explanation class. Perhaps change form_entity to field.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

I never understood how everyone knew about fieldset and not label. Label is like, the first thing you're supposed to learn when studying into accessibility.
Charles256
DevNet Resident
Posts: 1375
Joined: Fri Sep 16, 2005 9:06 pm

Post by Charles256 »

Code: Select all

<form id='login' action='?login' method='post'>
			<fieldset>
				<legend>
					<span class='legend'>
						User Login
					</span>
				</legend>
				<div class='form_entity'>
					<label class='form_explanation' for='username'>
						Username
					</label>
					<span class='form_input'>
						<input type='text' name='username' id='username' />
					</span>
				</div>
				<div class='form_entity'>
					<label class='form_explanation' for='password'>
						Password
					</label>
					<span class='form_input'>
						<input type='password' id='password' name='password' />
					</span>
				</div>
				<div class='form_entity'>
					<input type='submit' class='submit_button' value='Login' />
					<input type='reset' class='reset_button' value='Reset Form' />
				</div>
			</fieldset>
		</form>
That's what I came up with after a bit more work. Anymore suggestions?
User avatar
Ollie Saunders
DevNet Master
Posts: 3179
Joined: Tue May 24, 2005 6:01 pm
Location: UK

Post by Ollie Saunders »

I actually prefer <button> to <input type="submit"> et al. You can style it separately more easily and put markup in it. If you are using accesskeys you can markup a button like this:

Code: Select all

<style type="text/css">/*<![CDATA[*/
button kbd {
    font-family:inherit;
    text-decoration:underline;
}
/*]]>*/</style>
<button type="submit" accesskey="s"><kbd>S</kbd>umbit</button>
although apparently access-keys are a bad idea now, for some reason.

I see no reason for the <span class="form_intput"> tags and I would also use camelCase for my class names. The span in legend is a good idea for rendering purposes but the class is redundant. You do generally seem to be using a lot of classes, I try to minimise the class vocabulary and reuse the names, you can often ascertain mean through context; learn your CSS selectors. For instance, this is very useful:

Code: Select all

input[type=text], textarea {
   /* text fields only */
}
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Definitely throw away (almost) all classes. They are redundant, probably. You can access everything by

Code: Select all

#login fieldset {}
#login label {}
#login input {}
etc 
Also, the span in legend, do you need that?

Last, you could think about changing the divs to p's or even li's. But that's debatable/personal preference.
Charles256
DevNet Resident
Posts: 1375
Joined: Fri Sep 16, 2005 9:06 pm

Post by Charles256 »

The span in legend is needed if you try to style it some weird bugs pop up between i.e. and firefox that span is supposed to solve those issues. I haven't tested it to see if that was true but a guy on sitepoint referred me to an article describing it.
Post Reply