Masabi Rail Extension in dev release not updating form values after picking options from panel select

[GoogleCodeExporter]

What steps will reproduce the problem?
1.In the rail ticket app go to Buy a ticket>New Journey
2.Try out the inputs using select with panel class

What is the expected output? What do you see instead?
All selects convert to panel and display proper when you click them,
but only the To, From, and Date fields update properly on the form
when returning from picking option. All of the others just continue to
display the original selections and I'm not sure if the hidden form
inputs the script adds are getting updated or not.

What version of the product are you using? On what operating system?
iui-0.40-dev2.zip  on Windows with Firefox 3.6 and Safari 4.0.3 and Mobile
Safari on iPhone OS 3.1.2

Please provide any additional information below.
I am pretty certain these issues were not present when iUi dev.4 rel2 was
first released and have not modified any files from the zip. I even
re-downloaded and tested just to be sure.

Original issue reported on code.google.com by vichuds...@gmail.com on 31 Jan 2010 at 8:50

[GoogleCodeExporter]

Here is explanation of problem and fix. 

Basically part of the extension was broken when its blur and focus capturing was
changed to make it more external with less mod to iUI core js. After much 
firebug
console debugging, I determined there was no page blur being fired after 
clicking one
of the new SELECT converted to RADIO options. The original author added several 
blur
and focus event captures to the iUI showPage function and then used them as a 
trigger
to save and reload form values going from page to page. The function in 
iui_ext.js
that renders select inputs with a panel class added, used these events to 
trigger the
function to save your choice from the radio group to update the main form after 
going
back to it.

After I finally realized what was going on the fix was simple.
somewhere around line 286 in the iui_ext.js The last line of the 
convertSelectToPanel
function needs a small modification, as shown below, calling the authors 
function to
store form inputs before leaving the page.

------original--------------------------------------------------------------
m.onclick = n.onclick = function() { (this.lastChild ? this.lastChild :
this).checked=true; back(); return false; };

------change to-------------------------------------------------------------
m.onclick = n.onclick = function() { (this.lastChild ? this.lastChild :
this).checked=true; storeFormTags(generated,"input"); back(); return false; };

pretty cool extension. I have attached my modded file if some would rather just
download. The change above is the only one I made from the iui .40 dev 2 
release.

Original comment by vichuds...@gmail.com on 6 Mar 2010 at 11:36

Attachments:

• iui_ext.js

[GoogleCodeExporter]

Hey Vic,
Thanks for the debugging and the proposed fix.  If I understand your analysis 
correctly, it looks like I "broke" 
something when I integrated the masabi changes into iui.js.  

So should I apply your fix or should we look into why the onBlur isn't being 
called?  Do you know why onBlur 
isn't being called?  In the case you observed should onBlur be called?   Is 
injectEventMethods() being called for 
the page in question? 

Original comment by msgilli...@gmail.com on 9 Mar 2010 at 8:29

[GoogleCodeExporter]

From what I can understand, keep in mind I'm still in a little over
my head, as the author intended when he wrote the extension and modded
iui.js to fire focus and blur events, his event handler functions will
save any form values on  page blur and when a new page transitions in
if form fields are found with the same name they get filled in with
the stored values on page focus. The fix I made works well but the
missing blur event puzzled me.

 I don't think it's iui though. Since that fix I have been working on
the backbutton hacking issue we talked about on that thread as well.
My progress so far leads me to think it may be in the assignment of
event listeners in the extension or the fact that he uses history.back
after you click the panel select option. Perhaps it just needs changed
to call iui.back() instead. I didn't think of that till yesterday but
haven't tried it yet.

I think it's one of those two becuase the extension I'm working on is
based on adding flags to cause overrides to some standard iui
behavior, like the back button based on page content and needed web
app flow. Based on your advice I began looking for a way to put this
info in a page w/out using non standard attributes. What I've come up
with is using hidden form inputs in the page. What I've done so far is
modify the iui-event-log.js to watch for focus and blur of the page.
It took a while to get started becuase I am completley new to event
listeners an handlers. So far I have it detecting the hidden inputs
and it's working without fail for blur and focus on pages that are
internal links. I am going to play with some Ajax links after I get
home from work. I think it will be fine though. I am using iui.js
straight out of the box so I don't think the problem with panel select
lies there. I will upload the extesion when I get it working and if
you'd like to see what I have so far just let me know.

I have a few questions you may be able to help with though. Just about
everything is handeled by an externl js.  The only thing I think I
need to mod in iui.js to get my extension working is adding a back
parameter to showPageByHref and maybe showForm to be passed to the
slide functions like showPage has. Any others?

About adding event listeners. addEventListener( 1, 2, 3). I know 1 is
event you want to listen for, 2 is what to do on event, usually a
functio, I have seen examples with 3 being passed as true or false.
What is that parameter for? Thanks!

Vic Hudson

Original comment by vichuds...@gmail.com on 9 Mar 2010 at 9:20

[GoogleCodeExporter]

New Update

Here is whats going on.

The AZ list class and date picker continued to work because.....
AZ list
function convertSelectToSortedList(select) adds event listener (function 
azClick(e))
to the dom generated UL > LI > A which stores the chosen link with setItem:
function(key,value)

date picker
function dpSelect() which is called when you click a date also uses setItem:
function(key,value) to store chosen link

panel select class broke because ....
It uses dom created page to show converted select to a radio group. The created 
page
never gets any event listeners assigned to the radio group, nor does that page 
have
blur or focus event listeners assign to it. It depended on the mods he made to 
iui.js
to send the blur and focus and trigger his setItem function to save the chosen 
radio
group option.

Possible fixes.......
A)The one I already submitted - makes the least amount of change to iui_ext.js 
and
none to iui.js - also the other two classes have there own events or function 
calls
to save the values anyway, so giving that class it's own direct call like I did
doesn't seem like much stretch.
B)In iui_ext.js create an event listener and handler function for click events 
on the
radio group choices - adds more bloat to a sizable extension already

In short his main event handler function injectEventMethods(page) rewrites 
select on
page load and after ajax insert, but depended on his custom blur and focus 
events
that he put in iui.js to recall injectEventMethods(page) to store the form 
values
(Rerunning the whole function seems like overkill to me when you just need the 
part
to store form values). It looks like now iui.showPage(...) sends blur and focus
already so there's no problem there, but again without adding more bloat to a 
sizable
extension to make it listen for them it doesn't use them, so I suggest fix A as 
I
already submitted.  Please correct me if any one finds I'm wrong on that. I'm 
still
learning all this so anyone's feedback or opinion is appreciated.

Original comment by vichuds...@gmail.com on 10 Mar 2010 at 8:38

[GoogleCodeExporter]

Also, after I got TbBMod working I realized that if it called his form value 
save and
replace functions on all page blur and focus it could break the way my, and 
possibly
other extensions, work as TbBMod depends on form values that could be 
overwritten by
the masabi methods from page to page. 

Maybe masabi methods should get a check added so they leave hidden form inputs 
alone?
Until then, I definitely vote for the fix I already submitted!

;-)
Vic

Original comment by vichuds...@gmail.com on 11 Mar 2010 at 2:15

[GoogleCodeExporter]

At this time I still recommend the fix I provided. I'm concerned about the side
effect of iui_ext overwriting unintended form fields if we fix it to save all 
forms
on blur, and restore fields with the same name on the focus of another page as 
the
author intended. In addition to that iui_ext is also a substantial download for 
a 3g
or edge connection, I believe around the same size as iui, especially if your 
only
using part of the extension. What I propose to fix both issues, and intend to 
tackle
if I get time, is to bust it up into 4 smaller extensions.

formSticky ----- or something like that. This will be the part of iui_ext that 
saves
and restores form inputs on blur and focus with one small change, "only" inputs 
with
class = "sticky" will be carried from page to page if inputs have the same name.
Attach extension to page and just make inputs with the sticky class as needed. 
This
could be useful to some all by itself.

In the way I have it planned this these next 3 would all be stand alone (no 
mods to
iui.js), but would need formSticky attached to page as well, as they depend on
carrying form values from page to page. I don't really like having a 
dependency, but
it would be the only way to separate the functions of iui_ext without having to
include the form saving and restore feature coded into all extensions. The 
functions
would only need slight modification to use class = "sticky" for panelSelect and 
the
datePicker and azLister use stickyForms  function calls to save selection just 
as
iui_ext does now.

datePicker------- works as author wrote but is separate extension
azLister------ works as author intended but is separate extension 
panelSelect ------ works as author intended but is separate extension 

Questions, comments, ideas?

Vic Hudson

Original comment by vichuds...@gmail.com on 16 Mar 2010 at 12:04

[GoogleCodeExporter]

Given IUIs goal of a native-like ui, I'd like to see
panelSelect eventually become part of the core iui.js

"Sticky form" feature (if I understand correctly) is better handled
on the server side, in my case anyway.

Just my thoughts after reading this thread,hopefully I'll get to exploring the 
actual code this week. 

Original comment by lictor4@gmail.com on 16 Mar 2010 at 1:00

[GoogleCodeExporter]

I agree panel select should eventually become standard. I would also agree that 
in most cases sticky 
form is best done on server side. The extension in question however works by 
taking an HTML form 
select input with class of panel and writes a new node with the select options 
as a radio group and then 
adds it to the dom and replaces the original select with a hidden form input 
and a link to the new panel. It 
uses the sticky form behavoir I mention so when you click the link the radio 
group has what ever was in 
the hidden input selected. After you make a choice you go back to the original 
page and the hidden input 
gets your selection put in it. Hence sticky forms. Also several iUI webapps 
have several "pages" all in one 
file and you may not need a server call between the two pages but would like to 
have values carried. 

Original comment by vichuds...@gmail.com on 16 Mar 2010 at 1:22

[GoogleCodeExporter]

Aww haha hence "code before you comment".
Thanks for the recap

Original comment by lictor4@gmail.com on 16 Mar 2010 at 1:50

[GoogleCodeExporter]

Aww haha hence "code before you comment".
Thanks for the recap

Original comment by lictor4@gmail.com on 16 Mar 2010 at 2:19

[GoogleCodeExporter]

Aww haha hence "code before you comment".
Thanks for the recap, I'll have a look first hand this week

Original comment by lictor4@gmail.com on 16 Mar 2010 at 2:20

[GoogleCodeExporter]

No prob.

:-)

Original comment by vichuds...@gmail.com on 16 Mar 2010 at 2:27

[GoogleCodeExporter]

@Vic - I committed your fix as is.  I still want to know if there is a bug in 
iui.js regarding onBlur.  If there is, we 
should fix it.  If that fix causes unintended overwriting of form fields, we 
can fix that in iui_ext.js.

I like your idea of splitting it up into 4 separate extensions.  It is really 
big and I think there will be plenty of users 
who only want  one or two of the features.  Another suggestion that I have is 
that the implementation of HTML5 
sessionStorage functionality (setItem, getItem, etc) should default to a 
built-in implementation of supported by 
the browser.  This may also be a separate extension, or if small and useful 
enough, could go into core iui.js.

Original comment by msgilli...@gmail.com on 16 Mar 2010 at 6:13

• Changed state: Started

[GoogleCodeExporter]

@ Sean - Thanks for the feedback. I know you've been busy lately and I 
appreciate it.

 I don't believe the fault is in iui.js. When I made TbBMod I started with the
iui-event-log.js and built completely on that with no change to iui.js at all. 
It
seems to catch all page blur and focus events very reliably.

I said somewhere up the chain that it may be the fact that he uses history.back
instead of iui.back and that may be where the fault lies. It may be that 
simple, but
I haven't tried that yet. It could also be in the event Listener? 

I'm not opposed to change of either if it fixes iui_ext.js behavior, but if we 
go
that route I think we should modify the get and set item functions so that they 
only
get and set inputs with class = "sticky" and then modify the panel select, az 
list,
and if applicable date picker so that when it rewrites the original html 
(select and
date inputs with the appropriate class) and replaces with them with links and 
hidden
inputs, the hidden inputs get the sticky class as well. This would insure that 
only
the inputs used for this extension get changed and no other form inputs could 
get
overwritten by mistake. This is a more substantial retooling, unnecessary if 
we're
breaking it up into smaller extensions anyway?

I like the HTML5 idea. If we bust it into my 4 functions, stickyForms could be
designed to use HTML5 when browser's support it or fall back on homemade js 
storage
when not.  I like one method available to users, with an abstract background on 
how
its handled being irrelevant, and still giving the same results. Thoughts?

I'm also not sure how I feel about the last three requiring stickyForms to be
included. My Goal for extensions is for user to simply link a new js and/or css,
maybe add some extra "valid" HTML tags to page, and viola! Something about 
saying
this ext also requires this ext just doesn't sit right, but don't know how to
separate the functions into smaller extensions w/out doing that or adding the 
same
get/set functionality into all three extensions. Redundant????? I'd really like 
other
thoughts on that.

Of course if we built stickyForms into the core that problem solved.

;-)
Vic

Original comment by vichuds...@gmail.com on 16 Mar 2010 at 7:13