On 7/18/10 4:45 AM, RobG wrote:
The point of many of the criticisms (such as adding listeners to the
window object, using get/setAttribute instead of DOM properties, using
non-standard properties instead of classes) is that there are more
robust ways of doing the same thing that are standards compliant and
therefore far less subject to future failure.
I understand and would like to fix many/most of these things over time.
(However, the chance that any of these things will effect someone in the
browser on a modern, touch-capable, mobile device that has any real
market share is almost indistinguishable from zero.) iUI has had some
real issues like critical sections, poor transitions performance, broken
form handling, and lack of extensibility. These are the issues that
actually affect people and I have tried to focus on them.
I'm also a less experienced Javascript coder than Joe Hewitt and am
reluctant to change his code unless I'm fixing a bug or adding something
of clear value to users. I'm not opposed to this kind of cleanup, it's
just not on the top of my priority list. I'm also afraid that it might
break something. It would be great if someone who has your level of
knowledge would be willing to take the time to understand the complete
codebase, the backwards-compatibility issues our users would face when
upgrading, make some patches via Mercurial, test them (perhaps even
create some test cases and/or unit tests) etc.
That was't from the review (which is what I was referring to)
Yeah. It was the one I read after the jQTouch/Sencha one, because I
just search comp.lang.javasript for iUI to see what people were saying
over there.
, so I'll
presume you couldn't find an inaccurate statement there.
I didn't look, but when I looked at it a while back it seemed generally
accurate, but perhaps a little nit-picky.
All the
criticisms (of that version) stand and the authors would do well to
take note and correct them. After all, it's free advice and all of it
is technically sound, even if delivered with cod liver oil rather than
the more palatable sugar.
I'll drink cod liver oil if it'll make my code better ;)
You say it's purely cosmetic,
I said cosmetic, but I didn't really mean purely cosmetic. I mostly
understand the issues.
There is a method of declaring a global variable that is absolutely
guaranteed to work in any user agent that implements any version of
ECMAScript, it even uses fewer characters:
var iui = {...
I just fixed that one in my active clone that I'm doing the iPad work in:
http://code.google.com/r/msgilligan-iuiscroll/source/detail?r=115d6b9bf7544a914ee92fc72a0dbeb0aad5e0e1
Look good?
While there isn't a browser I know of whose window object can't be
augmented, there is no reason to believe one does't exist or won't
exist in the future.
But realistically what are the odds of a mobile device that breaks
adding things to window gaining any real market share?
So given the choice of code that is guaranteed to
work for any ECMAScript implementation now and into the future, or
code that may fail at some future point purely for the sake of not
following standards, why on earth would you choose the second?
I see that the following has also survived:
| if (!child.id)
| child.id = "__" + (++newPageCount) + "__";
Here the id property is read and, if it doesn't exist, a (more or
less) random value is added. For what purpose?
iUI keeps a navigation stack ("pageHistory") that stores the id of each
node that is pushed on the stack as the user navigates down. If a node
doesn't have an id, one is added.
| var clone = $(child.id);
The $ function is simply a wrapper for getElementById. So this
statement uses the id (either read from the element or added to it)
with getElementById to return exactly the same element.
Actually, if you look more closely at the code, you'll see that child
is not a node in the DOM but has just been created with
document.createElement() This code makes sure that if the new node has
an id that's already taken that the old one is removed first.
| if (clone)
Under what circumstances will clone *not* exist? The loop started with
an element, then used getElementById to return exactly the same
element.
See above.
| clone.parentNode.replaceChild(child, clone);
Now the element is replaced by itself. What is the purpose of that?
See above.
| else
| document.body.appendChild(child);
Under what circumstance is this statement ever executed? Presumably,
where clone doesn't exist and getElementById has returned null. But if
that was true, surely child doesn't exist either? If that were so,
this loop would not be executed at all (for various reasons).
This is the case where the node doesn't exist and has just been loaded
via Ajax for the first time.
| if (child.getAttribute("selected") == "true" || !targetPage)
| targetPage = child;
Now DOM properties are eschewed in favour of the (known to be buggy)
getAttribute method.
Buggy in IE (which is not supported) or buggy in browsers we care
about? Do you have a link with more info on this?
So you're saying it would be better to just say (child.selected ==
"true") ??
Further, for those elements where the selected
attribute is specified in the W3C DOM HTML specification, it's defined
as being a boolean and will return boolean true, not the string true.
So this code will *only* work on an element that *doesn't* have a
selected attribute specified by the relevant standard.
I think the only element that can have a W3C attribute named @selected
is the <option> element which would never be the top element of an iUI
"page". I've never heard of, or seen the need for iUI pages on other
than <div>, <ul>, and <form> elements. But <option> would never be
used, because it should always be inside a <select> which should
probably be inside a <form>.
I agree that the choice of using a @selected attribute rather than a CSS
class is less-then-ideal, but that is pretty pervasive to the code (and
existing apps) and has caused zero problems to my knowledge (and I do
hear about most problems) At some point we'll probably add a
class-based alternative and deprecate using "selected"...
Again, a safe and standards compliant alternative is available at zero
extra effort - the class attribute.
... but this won't be at zero effort to me or the community.
In the latest code, I've deprecated the @orient attribute on the <body>
tag in preference to using classnames. I did this partly to address a
bug, but I am trying to slowly move away from non-standard attributes.
It's not high on the priority list, though.
It is these types of basic faults that can be addressed at very little
effort and result in a much more robust script.
I would like to address these issues over time and very much appreciate
your constructive criticism -- Thanks!
If you're feeling ambitious and/or generous, feel free to clone iUI in
Mercurial and create a fixed-up version that we can look at. I promise
to take a hard look at it.
Thanks!
-- Sean
--
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.
