Instead of continually pelting the list with diatribes about how much IUI sucks, why don't you fork the project and fix it up to your standards? It's mirrored up on github at http://github.com/pjhyett/iui/tree/master - takes 20 seconds to sign up for an account and another 10 to fork the project. That way everybody wins.
On Wed, Jan 7, 2009 at 1:41 PM, 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. > > var slideSpeed = 20; > var slideInterval = 0; > > var currentPage = null; > var currentDialog = null; > > var currentWidth = 0; > var currentHash = location.hash; > > Implied global. Use window.location.hash. > > var hashPrefix = "#_"; > var pageHistory = []; > var newPageCount = 0; > var checkTimer; > > // > > ************************************************************************************************* > > 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. > > { > showPage: function(page, backwards) > { > if (page) > { > > Sloppy. Why would the code call this with a "falsy" page argument? > > if (currentDialog) > { > currentDialog.removeAttribute("selected"); > > So - currentDialog - is a reference to an element. Apparently > "selected" is an expando, which is a bad idea. > > currentDialog = null; > } > > if (hasClass(page, "dialog")) > > Will be interesting to see the latest re-invention of this particular > wheel. > > showDialog(page); > else > { > var fromPage = currentPage; > currentPage = page; > > if (fromPage) > setTimeout(slidePages, 0, fromPage, page, > backwards); > > Implied global. Use window.setTimeout and don't pass arguments like > this. And assuming this is some sort of timed effect, why should it > be called from a timeout? > > else > updatePage(page, fromPage); > > The missing curly brackets are a mistake (especially for an open > source project with multiple contributors.) > > } > } > }, > > showPageById: function(pageId) > { > var page = $(pageId); > > The "$" function is silly (and always has been.) > > if (page) > { > var index = pageHistory.indexOf(pageId); > > Compatibility issue. > > var backwards = index != -1; > if (backwards) > pageHistory.splice(index, pageHistory.length); > > iui.showPage(page, backwards); > } > }, > > showPageByHref: function(href, args, method, replace, cb) > { > var req = new XMLHttpRequest(); > req.onerror = function() > { > if (cb) > cb(false); > }; > > req.onreadystatechange = function() > { > if (req.readyState == 4) > { > if (replace) > replaceElementWithSource(replace, > req.responseText); > else > { > var frag = document.createElement("div"); > frag.innerHTML = req.responseText; > iui.insertPages(frag.childNodes); > } > if (cb) > setTimeout(cb, 1000, true); > > A one-second delay on the callback? > > } > }; > > if (args) > { > req.open(method || "GET", href, true); > req.setRequestHeader("Content-Type", "application/x-www- > form-urlencoded"); > req.setRequestHeader("Content-Length", args.length); > req.send(args.join("&")); > } > else > { > req.open(method || "GET", href, true); > req.send(null); > > Repetitious. > > } > }, > > insertPages: function(nodes) > { > var targetPage; > for (var i = 0; i < nodes.length; ++i) > > Inefficient loop condition. > > { > var child = nodes[i]; > if (child.nodeType == 1) > { > if (!child.id) > child.id = "__" + (++newPageCount) + "__"; > > var clone = $(child.id); > > Think about that one for a moment ($ is just a wrapper for gEBI.) > > if (clone) > clone.parentNode.replaceChild(child, clone); > > So this replaces the child with itself (not a clone of itself.) > > else > document.body.appendChild(child); > > 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.) > > targetPage = child; > > --i; > > This is an odd loop indeed. > > } > } > > if (targetPage) > iui.showPage(targetPage); > }, > > 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. > > 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--) { > ... > } > > }; > > // > > ************************************************************************************************* > > addEventListener("load", function(event) > > Implied global. That's the first time I've seen that one. And it > would be more standard to attach this listener to the document body. > > { > var page = iui.getSelectedPage(); > if (page) > iui.showPage(page); > > Then why does showPage check for a truthy page argument? > > setTimeout(preloadImages, 0); > setTimeout(checkOrientAndLocation, 0); > checkTimer = setInterval(checkOrientAndLocation, 300); > > Implied global hat trick. > > }, false); > > addEventListener("click", function(event) > { > > Is this really meant to be a click listener on the (implied) window > object? That's clearly a mistake (leave the window object alone.) > > var link = findParent(event.target, "a"); > > It really is. Delegation from a window listener. Don't try *that* at > home. > > if (link) > { > function unselect() { link.removeAttribute("selected"); } > > Don't nest function declarations (and don't create them every time the > user clicks!) > > if (link.href && link.hash && link.hash != "#") > { > link.setAttribute("selected", "true"); > > As mentioned, this is a ludicrous design. > > iui.showPage($(link.hash.substr(1))); > setTimeout(unselect, 500); > } > else if (link == $("backButton")) > history.back(); > > Why duplicate the back button? There is no way to enable or disable > the faux back button, so it makes little sense from a usability > standpoint. > > else if (link.getAttribute("type") == "submit") > submitForm(findParent(link, "form")); > else if (link.getAttribute("type") == "cancel") > > This thing really plays fast and loop with the DOM. Outside of this > warped universe, what sort of links have such type attributes? > > cancelDialog(findParent(link, "form")); > > And what sort of dialog interface would use links for buttons? > > else if (link.target == "_replace") > { > link.setAttribute("selected", "progress"); > iui.showPageByHref(link.href, null, null, link, unselect); > } > else if (!link.target) > { > link.setAttribute("selected", "progress"); > iui.showPageByHref(link.href, null, null, null, unselect); > } > else > return; > > event.preventDefault(); > } > }, true); > > addEventListener("click", function(event) > { > > Again?! > > var div = findParent(event.target, "div"); > if (div && hasClass(div, "toggle")) > { > div.setAttribute("toggled", div.getAttribute("toggled") != > "true"); > > This seems like a complete waste of time. > > event.preventDefault(); > } > }, true); > > 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? > > 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. > > } > > if (location.hash != currentHash) > { > var pageId = location.hash.substr(hashPrefix.length) > iui.showPageById(pageId); > } > } > > function showDialog(page) > { > currentDialog = page; > page.setAttribute("selected", "true"); > > Why? They stored a reference to the "current dialog." > > if (hasClass(page, "dialog") && !page.target) > showForm(page); > } > > function showForm(form) > { > form.onsubmit = function(event) > { > event.preventDefault(); > submitForm(form); > }; > > Why re-create this function every call? And why is this done with > DOM0? > > form.onclick = function(event) > { > if (event.target == form && hasClass(form, "dialog")) > cancelDialog(form); > }; > > Same and same. And why should clicking the form background hide it? > > } > > 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. > > function updatePage(page, fromPage) > { > if (!page.id) > page.id = "__" + (++newPageCount) + "__"; > > Seen that before. This is a tiny script, yet it has lots of > duplication. Is anybody minding this store? > > location.href = currentHash = hashPrefix + page.id; > pageHistory.push(page.id); > > var pageTitle = $("pageTitle"); > if (page.title) > pageTitle.innerHTML = page.title; > > if (page.localName.toLowerCase() == "form" && !page.target) > > Use nodeName instead. > > showForm(page); > > var backButton = $("backButton"); > if (backButton) > { > var prevPage = $(pageHistory[pageHistory.length-2]); > if (prevPage && !page.getAttribute("hideBackButton")) > { > backButton.style.display = "inline"; > > That should be part of a class (what if you don't want "inline?") > Alternatively, set the display style to "". > > backButton.innerHTML = prevPage.title ? prevPage.title : > "Back"; > } > else > backButton.style.display = "none"; > } > } > > function slidePages(fromPage, toPage, backwards) > { > var axis = (backwards ? fromPage : toPage).getAttribute("axis"); > if (axis == "y") > (backwards ? fromPage : toPage).style.top = "100%"; > else > toPage.style.left = "100%"; > > toPage.setAttribute("selected", "true"); > scrollTo(0, 1); > clearInterval(checkTimer); > > var percent = 100; > slide(); > var timer = setInterval(slide, slideInterval); > > function slide() > { > percent -= slideSpeed; > if (percent <= 0) > { > percent = 0; > if (!hasClass(toPage, "dialog")) > fromPage.removeAttribute("selected"); > clearInterval(timer); > checkTimer = setInterval(checkOrientAndLocation, 300); > setTimeout(updatePage, 0, toPage, fromPage); > } > > if (axis == "y") > { > backwards > ? fromPage.style.top = (100-percent) + "%" > : toPage.style.top = percent + "%"; > } > else > { > fromPage.style.left = (backwards ? (100-percent) : > (percent-100)) + "%"; > toPage.style.left = (backwards ? -percent : percent) + > "%"; > } > } > > That is going to be one hellaciously slow animation. Why aren't > people using those Web 2.0 wonder libraries for this stuff? Oh yeah, > they are too big, too slow and aren't cross-browser at all. So here > we are re-inventing animations. > > } > > function preloadImages() > { > var preloader = document.createElement("div"); > preloader.id = "preloader"; > document.body.appendChild(preloader); > > How very interesting. Leave the cache alone. > > } > > function submitForm(form) > { > iui.showPageByHref(form.action || "POST", encodeForm(form), > form.method); > } > > 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? > > var args = []; > encode(form.getElementsByTagName("input")); > > Don't use gEBTN to fetch form elements (use the elements collection.) > > encode(form.getElementsByTagName("select")); > > Same. > > return args; > } > > function findParent(node, localName) > { > while (node && (node.nodeType != 1 || node.localName.toLowerCase > () != localName)) > > Use nodeName. Test is ridiculous. > > node = node.parentNode; > return node; > } > > function hasClass(self, name) > { > var re = new RegExp("(^|\\s)"+name+"($|\\s)"); > > Don't create regular expressions endlessly. > > return re.exec(self.getAttribute("class")) != null; > > Use self.className and the test method. > > } > > function replaceElementWithSource(replace, source) > { > var page = replace.parentNode; > var parent = replace; > while (page.parentNode != document.body) > { > page = page.parentNode; > parent = parent.parentNode; > } > > God only knows what this is meant to be. I'm sure that further > investigation would be a waste of time. > > var frag = document.createElement(parent.localName); > frag.innerHTML = source; > > page.removeChild(parent); > > while (frag.firstChild) > page.appendChild(frag.firstChild); > } > > function $(id) { return document.getElementById(id); } > > Have we learned nothing from Prototype's mistakes? It's been what? > Four years? > > function ddd() { console.log.apply(console, arguments); } > > That is almost as bad a name as "$". > > })(); > > 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. > > > --~--~---------~--~----~------------~-------~--~----~ 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 -~----------~----~----~----~------~----~------~--~---
