Hi Michael.
Thanks for taking the time and effort to review the component so
thoroughly.
"Michael Winter" <M.Winter@blueyonder.co.invalid> wrote in message news:<opsgue4fsix13kvk@atlantis>...[color=blue]
> [Mailed to OP]
>
> On 1 Nov 2004 12:32:46 -0800, Oliver Bryant <waolly@gmail.com> wrote:
>[color=green]
> > I just finished developing a javascipt component allowing floating
> > captions to appear over HTML elements.
> >
> > If anyone wants to check it out you can see specs and download it from
> >
http://boxover.swazz.org.[/color]
>
> First of all, apologies to the OP. I'm mailing you as I'm not sure if
> you're still watching this group.[/color]
No worries about the mail, I am still watching the group though ;)
[color=blue]
> Secondly, the presentation is nice, but there are a couple of important
> comments I would like to make.
>
> One huge problem with this is that the title attribute is thoroughly
> raped. For your script to work, the author must enter text that meets a
> specific format. However, how will that be dealt with by accessibility
> devices like Braille and screen readers, or users with Javascript disabled
> but browsers that render the title content?[/color]
Couldn't work out if you were taking the piss here to start. I have
the deepest regard for handicapped people in any form, but I have no
idea how a Braille screen reader works and quite frankly don't think
that this is a major consideration when talking about a javscipt
component.
[color=blue]
> Authors that are required by law to produce accessible sites (a growing
> number) might like your script, but they will be completely unable to use
> it. The text for the caption can be specified by the content of the title
> attribute, but that should be it.[/color]
Boxover is very similar to a component called overlib. Overlib is
currently used on sites like NASA and HP - sites which would be under
the highest of scrutiny. So I disagree, people will be able to use
it. In my experience the most important factor is how many browsers a
javascript component will work under.
[color=blue]
> I also think you should be more selective when applying the effect. Every
> element with a title attribute seems over-the-top.[/color]
This is a good point. I will include a modification which will check
for a keyword siginifying whether a caption should appear or not.
[color=blue]
>
> The web site
>
> - You should validate your HTML and CSS.
>
> See the results for each at:
>
> <URL:http://validator.w3.org/check?uri=http%3A%2F%2Fboxover.swazz.org%2F>
> <URL:http://jigsaw.w3.org/css-validator/validator?uri=http%3A%2F%2Fboxover.swazz.org%2F&us ermedium=all>
>
> - Similar to above: please advocate good development practice.
>
> 1) <SCRIPT SRC="BoxOver.js"></SCRIPT>
>
> The SCRIPT element requires the type attribute. Update the snippet to:
>
> <script type="text/javascript" src="BoxOver.js"></script>[/color]
I think this is being overly pedantic. I've never had a problem with
not including the type attribute. From a theoretical (and possibly
legacy) standpoint, sure. But on the field this works in all browsers
I've seen.
[color=blue]
> 2) <DIV style="BORDER: #558844 1px solid;WIDTH:200px;HEIGHT: 75px">
>
> Pixels values should be avoided for dimensions that are related to text.
> Instead, use ems, which are proportional to the height of the current font.[/color]
Admittedly I don't have any experience with the "em" unit. However I
have designed tons of websites and the "px" unit has never done me any
wrong. But I'm not in a fair position to compare the two.
[color=blue]
> 3) Uses: ALT and TITLE replacements
>
> This code is *not* a replacement for the alt attribute. The alt attribute
> is a replacement used when the image, form control, applet, or client-side
> image map area cannot be rendered. It should not be implied that alt is
> for providing tool-tips. That's the job of the title attribute.[/color]
My apologies. I was originally considering making the component
configurable through the ALT tag as well. I have modified the site.
[color=blue]
>
> The script
>
> - You don't use feature detection.[/color]
Of course I use feature detection. How does it work in different
browsers then?
[color=blue]
> The first few lines of code look like (wrapped):
>
> document.all ? window.attachEvent('onload',init)
> : window.addEventListener('load',init,false);
>
> However, what does document.all have to do with support for
> window.attachEvent? Yes, IE supports document.all and the attachEvent
> method, but other browsers do support document.all but probably not
> attachEvent.
>
> See <URL:http://www.jibbering.com/faq/faq_notes/not_browser_detect.html>
> for more information.[/color]
The code in this component works in all major browsers. I did it
myself and I know it works. The browser detection at this link checks
all sorts of things indirectly.
I believe that checking the actual existence of a property or object
is better - you don't have to rely on some derived browser value
earlier and then use a redundant condition which separates code for
each of the browsers. This uses more code and relies on you having
detected the correct browser earlier. Checking the object directly
and then loading the appropriate object into a variable which can then
manipulated with the same logic is better in my opinion.
[color=blue]
> - You pollute the global namespace.
>
> Anyone that has written non-trivial code in any language will tell you
> that global variables, especially in library code, are a bad idea. If
> anyone happens to use a variable name that matches one of yours, its
> likely that either their script, or yours, will break.
>
> It's also quite curious that in some cases, you choose global scope when
> it should clearly be local.[/color]
I'm sure people will tell me all sorts of things - but they normally
apply to the general case. I designed this with low CPU usage in mind
and it is a hell of a lof more efficient using global variables.
Using local variables means recreating them each time an event is
fired - javascript is already pretty inefficient as a language. My
reasons for using global variables are justified with regards to
performance.
[color=blue]
> You might want to read a post I recently made on c.l.js
> <URL:http://groups.google.com/groups?threadm=opsgt8lmmvx13kvk%40atlantis>
> in the thread "Define prefix MM to function". That link may not be active
> yet.
>
> - Overuse of the style object.
>
> To be honest, I fail to see why you're applying the bulk of your styles
> though script when they could be applied through a class or id selector in
> a style sheet. Furthermore, there are several instances where you specify
> length values, but you fail to include a unit. This is invalid and
> conforming browsers should ignore those declarations.[/color]
The reason is simple. When somebody downloads boxover I want the
default manifestitation to look nice and be as easy to use as
possible. One souce file, one <script> tag, and altering a title
attribute - easy as that. Using classes would mean an extra
stipulation for the user to consider when setting it up (I couldn't
find any way to define a class inside the component source).
The component is designed so that a user can apply a style class to
any caption if they desire - but that might be out of scope for some
users so I thought defaulting to a nice looking default was best.
[color=blue]
> - Bad use of parseInt.
>
> See the FAQ notes regarding type conversion, particularly the section,
> "parseInt with a radix argument":
> <URL:http://www.jibbering.com/faq/faq_notes/type_convert.html>.[/color]
Fair enough. That is a good article - thanks for the link. I
normally use parseInt to ensure that mathematical expressions are
evaluated correctly, e.g. so that 1 + 1 = 2 and not 11 (if it
considers + to indicate concatenation of strings rather than numerical
addition). I don't like that fact that the article refers to it as
inefficient. This I will look in to.
[color=blue]
> - Declaring variables without using var.
>
> Related to an earlier point, you introduce some variables without using
> the var keyword. Most of these should only be local, so the var keyword
> must be used in such cases. However, I (certainly) regard it as good
> practice that all variables, global or not, should be declared with the
> var keyword.
>
> One last, much more minor point:
>
> curNode.hasbox='true';
>
> seems a little odd. Why not use a boolean?[/color]
Yeah quite right. I should have used boolean. Must have been one of
those late nights ;)
[color=blue]
> I wouldn't call this list exhaustive, but it would certainly resolve the
> majority of problems. I haven't for example, examined the positioning code
> moveMouse() in any great detail.
> Mike[/color]
I reckon the list is pretty exhaustive!
In conclusion, I appreciate your criticisms but I think that some are
unjustified. I spent hours on this component looking at all possible
trade offs. It has been designed to work in all major browsers -
Opera, IE, firefox, mozilla at very little CPU. These were my
strongest considerations when coding.
Thanks again for the in depth look you took into the component. I'll
be sure to ask your opinion on javascript matters in the future!
