On Jan 8, 6:41 am, Roger <[email protected]> wrote:
> Is it ever. And of course, its proponents will all chime in that I
> hate frameworks and like wasting money. You can listen to morons or
> you can listen to me.
>
> And be sure to read to the end as it was a journey. The biggest
> bombshell is about three quarters of the way through.
>
> Executive summary: You'd have to be completely mental to consider
> using this on a Website.
>
> /*
> Copyright (c) 2007, iUI Project Members
>
> That should give you pause.
>
> See LICENSE.txt for licensing terms
> */
>
> (function() {
>
> Assuming this is an iPhone/iPod script only (which should also give
> you pause), otherwise it would certainly leak memory in IE.
While this pattern has the potential to cause leaks (as the point of
using it is generally to use closures in lieu of global variables), it
is the use of closures with circular references that cause leaks in
IE. But this script doesn't work in IE anyway.
[...]
>
> var currentWidth = 0;
> var currentHash = location.hash;
>
> Implied global. Use window.location.hash.
Further down, you criticize the use of window.iui, presumably because
it infers the existence of a window object. If that is your position,
then:
var currentHash = window && window.location && window.location.hash;
seems suitable. However, in a script for visual user agents and
browsers, perhaps:
if (!window) return;
should be the first line of the script.
[...]
>
> window.iui =
>
> This is a mistake. The window object is a host object, so it should
> not be augmented. This also assumes that the global window property
> references the Global Object. The proper way to do this is to store a
> reference to the Global Object prior to entering the one-off function.
Or to get a reference to the global object from inside the function:
var global = (function(){ return this; })();
global.iui = ...
[...]
>
> if (hasClass(page, "dialog"))
>
> Will be interesting to see the latest re-invention of this particular
> wheel.
It seems to be a pretty standard implementation.
[...]
> if (fromPage)
> setTimeout(slidePages, 0, fromPage, page,
> backwards);
>
> Implied global. Use window.setTimeout and don't pass arguments like
> this.
Yes, extra parameters don't work in IE. Most developers of iPhone web
apps don't seem to consider other environments. I also get annoyed by
sites that force me to access their crappy iPhone version when I want
to see the full version of their site on my iPhone.
> And assuming this is some sort of timed effect, why should it
> be called from a timeout?
So that the rest of the function completes before any animation
starts.
[...]
> var page = $(pageId);
>
> The "$" function is silly (and always has been.)
Yes.
[...]
>
> if (child.getAttribute("selected") == "true" || !
> targetPage)
>
> The use of get/set/removeAttribute for these flags is ridiculous.
> Clearly they all have ID's, so why not store this information in an
> object (as opposed to invalidating the markup.)
Or store it in the class attribute, which is designed for such stuff.
[...]
> getSelectedPage: function()
> {
> for (var child = document.body.firstChild; child; child =
> child.nextSibling)
> {
> if (child.nodeType == 1 && child.getAttribute("selected")
> == "true")
> return child;
> }
> }
>
> This is what I'm talking about. There should be a flag of some sort
> for this. Looping through every child of the body is outrageously
> inefficient.
It may not be, the body element usually doesn't have that many direct
children.
>
> Also, as this loop is only concerned with element nodes, it would be
> far more efficient as:
>
> var childElements = document.body.getElementsByTagName('*');
> var index = childElements.length;
>
> while (index--) {
Ooops! The collection of body.childNodes will almost certainly be far
fewer nodes than the set of all elements in the body. But your point
is still valid - a reference to the 'selected' element should be
stored so it can be retrieved much more easily.
[...]
> addEventListener("load", function(event)
>
> Implied global. That's the first time I've seen that one.
And another reason why this will go belly-up in IE. What happens when
the site is supposed to work with Windows Mobile?
[...]
> if (link)
> {
> function unselect() { link.removeAttribute("selected"); }
>
> Don't nest function declarations (and don't create them every time the
> user clicks!)
It is, strictly, a syntax error, but various implementations allow it.
[...]
> function checkOrientAndLocation()
> {
> if (window.innerWidth != currentWidth)
> {
> currentWidth = window.innerWidth;
> var orient = currentWidth == 320 ? "profile" : "landscape";
>
> There's a bad assumption.
>
> document.body.setAttribute("orient", orient);
>
> No way to query this property?
There is window.orientation.
>
> setTimeout(scrollTo, 100, 0, 1);
>
> I hate this (and see it everywhere in scripts for this device.)
> Passing a host method to setTimeout is insanity.
Why, other than it is a DOM 0 method that might not be supported and
hasn't be properly tested for?
> }
[...]
> function cancelDialog(form)
> {
> form.removeAttribute("selected");
>
> }
>
> OMFG. Just realized why they are twiddling with non-standard
> attributes. The style sheets tell the tale (they have no clue what
> they are doing!) All of these calls to set/removeAttribute should
> instead be adding or removing classes.
Yes.
[...]
> function encodeForm(form)
> {
> function encode(inputs)
> {
> for (var i = 0; i < inputs.length; ++i)
> {
> if (inputs[i].name)
> args.push(inputs[i].name + "=" + escape(inputs
> [i].value));
> }
> }
>
> Not even close. Here we are re-inventing form serialization.
> Prototype, jQuery, etc. didn't really work out as advertised, did
> they?
It also doesn't check if the 'input' has been disabled, or if
checkboxes and radio buttons are checked.
[...]
> function findParent(node, localName)
> {
> while (node && (node.nodeType != 1 || node.localName.toLowerCase
> () != localName))
>
> Use nodeName.
localName is part of DOM 2, I guess you think it's not cross browser?
> Test is ridiculous.
Yes.
> node = node.parentNode;
> return node;
>
> }
>
> function hasClass(self, name)
> {
> var re = new RegExp("(^|\\s)"+name+"($|\\s)");
>
> Don't create regular expressions endlessly.
It has to be created 'endlessly', how do you use a regular expression
that matches the value of a string passed to the function otherwise?
I would probably have used "cName" rather than "name".
[...]
> Not a single comment. Currently patched by whomever. Whole idea is
> ill-advised (Ajax navigation and emulating the default native
> interface of the iPhone.) Design is ridiculous (see the CSS for more
> info.) Implementation is botched (see above.) What is the point of
> leaning on this thing? Have developers become so conditioned to
> leaning on other people's do-everything-for-me scripts that they will
> jump on any old thing? And old is the operative word. This was
> slapped together two years ago (for whatever reason) and is still
> completely lacking today.
All correct. Maintenance and further development is a big issue for
such libraries.
--
Rob
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups
"iPhoneWebDev" group.
To post to this group, send email to [email protected]
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/iphonewebdev?hl=en
-~----------~----~----~----~------~----~------~--~---
