Forum OpenACS Development: Form-builder display/edit modes

Collapse
Posted by Lars Pind on
I've made an enhancement to form-builder, which I'm ready to commit
to the repository.

What it does is let you add a "-mode display" switch to your form,
in which case it'll be displayed in "display" mode, meaning that
instead of an input/select/whatever widget, the value is displayed
in straight HTML. You can also set "-mode edit" or "-mode display"
on each individual element in a form, thus overruling the default
for the form.

I developed it for a client project, but it's also very useful for
applications like the bug-tracker (http://clients.museatech.net/bug-
tracker/), where I like to display the bug form with different
fields enabled, depending on the action you're taking.

It's better than using inform widgets (which is what I did for bug-
tracker) because it's annoying when you have a select widget where
the value isn't the same as the label. The enhancements I've made
changes each widget to display itself differently depending on
whether it's in display or edit mode.

Another small change is the ability to add control the number, name,
and labels on the buttons below the form, instead of only having one
with a hard-coded label of "submit". You do this by saying "-buttons
{ { "OK" ok } { "Cancel" cancel } }" when you create the form. If
you don't say -buttons, the default submit button will be created.

This is useful if you like the OK Cancel convention that I happen to
like. Or if you need more buttons as in the bug-tracker (Edit,
Resolve, Close, Reopen, etc.). Handling what happens when you click
those other buttons is entirely up to you and how you handle your
form submits.

This touches a lot of places in the form builder code, so there
might be unintended side-effects that I didn't test for, e.g. with
dotLRN, or with Don's ad_form stuff.

The question is: Any objections to me committing this now? Should I
wait a bit to not obstruct any development efforts going on? Or will
you be able to handle it allright? I know, we're using a version
control system, but it's always annoying when things inadvertently
break (not that I think they will) :)

Gimme the word, and if I don't hear back soon, I'll commit it.

/Lars

Collapse
Posted by Don Baccus on
dotLRN's supposed to deploy in a couple of weeks and OF and furfly are working actively on the OpenACS tip so if your code were to break things a few of us might be unhappy.  OF uses the form builder heavily (yay!) and furfly has too in our two packages.

How well has this been tested?

I've got some trivial commits of my own to make to acs-templating, including making "date" a real type rather than simply a funky widget (it uses the same widget so everything's backwards compatible).  I need these for my (modest) dotLRN work.

I like the "-mode display" addition.  ad_form currently supports "-confirm_template" which allows you to specify a template in the "OK Cancel" style displayed for confirmation before ad_form executes the "-new_data" or "-edit_data" clauses.  Your "-mode display" gives ad_form a way to generate confirm pages without the need for the user to write their own confirmation template, and that's cool.  A user will then be able to use "-confirm" if they're lazy/satisfied with the automatically generated page, "-confirm_template" if they want more control.  Yeah, sounds pretty cool alright!

"-buttons" is good, too.  I did something similar for Greenpeace but made a "button" datatype with widget instead of adding a special switch.  We might want to compare notes before deciding which approach is best.  Something that allows us to generate multiple buttons is certainly needed.

The only advantage of having it as a type is that the user can insert it anywhere in the form, and can use it with standard element stuff like "html" (think JavaScript).

But my Greenpeace code for this is a bit of a hack, IIRC ...

Maybe you and I can test this a bit more, decide which "button" approach is best, and I can add some integration with ad_form and then we can commit it?  In (say) the next week or so?

Collapse
Posted by Lars Pind on
Peter's installed the latest version of .LRN today, so we can at least test it with that before committing.

It's been used extensively on a client project for over a month, without any hiccups, but I think the issue would mainly be with widgets that I overlooked, because I didn't happen to use them myself.

I can upload the patch and let you and others try it out before I commit. How's that?

/Lars

Collapse
Posted by Don Baccus on
Actually if you test it with dotLRN that would be fine with me.  I trust you guys :)

How about the button issue?  If you have a copy of GP Planet that's up-to-date you can look at gp-tcl/tcl/form-processing ...

Collapse
Posted by Don Baccus on
buttons ... hmmm ... one reason I went the type/widget approach is that the form builder already had a "submit" widget (or type? I need to look).  I think this approach fits in as they really are form elements.

Anyway I'll try to spend a little time this weekend looking at my GP button type/widget hack to see just how ugly it is :)

Collapse
Posted by Lars Pind on
Alright, just committed this now, only six months late :)

It's on the HEAD. Use like this:

form create ... -mode display

Let me know what you think.

/Lars

Collapse
Posted by xx xx on
Great, this sounds very, very useful.
Did you fix the 'cancel' button too (I mean, will it cancel the action, since it doesn't in 4.5)?
Collapse
Posted by Lars Pind on
Not really. I made Cancel not on by default. But you still have to manually catch the cancel button and do a redirect in your Tcl file.

I don't see any other way. Of course, I could make sure that [form is_valid ...] returns false, but the form by default doesn't know where you should go in case of a cancel.

Hm, of course we could add that as another switch to the form element.

form create -buttons [list [list ok OK] [list cancel Cancel $return_url]]

?

/Lars

Collapse
Posted by Tilmann Singer on
What about something like:
form create edit \
  -cancel_url $return_url \
  -cancel_label "Cancel"
where cancel_label would be "Cancel" by default or the i18n-ised default, and the button would only appear if cancel_url is specified? This would fit in better with the self-describing api of the form builder IMHO.
Collapse
Posted by Roberto Mello on
I backported lars' changes (which are ver nice btw) to 4.6 on my personal checkout (just involved removing some calls to lc_time_fmt in the date procs).

It runs fine, but after I edit and submit, the values for the radio buttons and checkboxes don't show. Did I miss something?

While we're on the note, does someone have examples of using the wizard stuff for the form-builder? It seems so darn useful, but without examples it makes it hard.

Thanks,

-Roberto

Collapse
Posted by Jun Yamog on
Hi Roberto,

I used wizard extensively on one of my package.  I can tar it up to you so you can see how its used.  Its damn useful.  I also submitted a very very small patch in the SDM on it.

Just tell me if you want a tar ball of my package and I will email it to you.

Collapse
Posted by Roberto Mello on
Jun,

Thanks for your tarball. I'll look at it.

Where's this patch you submitted to the SDM? I looked for it but couldn't find it (didn't look very far, admittedly).

-Roberto

P.S: I found out why the images for checked/unchecked were not being shown in display mode: they are not in 4.6, so I had to copy them from HEAD.

P.S2: I modified ad_form so it works with Lars' -mode switch to the form builder, and also so elements accept the "section" attribute. They are both one-liners. Should I commit this to HEAD or submit a patch?

Collapse
Posted by Jun Yamog on
Hi Roberto,

Jeff Davis has accepted it, should be in 4.6 already.  Its just a simple html bug.  That should display the wizard steps/progress on right side properly.

Also if you would notice in my wizards I resuse the forms in the steps.  For example on wizards that needs to pick a content folder.  I just use folder-list.tcl on it.  So picking an image, page, file or folder uses the same ad_form over and over again.  Almost all ad_form are reused, so I have to be careful not to duplicate the state variables names.  If I 2 or more ad_forms that sets the same state var then the lastest form to set it will overwrite the previous ones.

Collapse
Posted by Lars Pind on
Tilmann,

I've implemented your suggestion now:

Added -cancel_url and -cancel_label switches to form::create.

If you supply cancel_url but no cancel_label, the default of "Cancel" will be used. (That should be internationalized, I know).

So now the form builder can handle the cancel button for you automatically.

/Lars

Collapse
Posted by Ola Hansson on
Cool, Lars!

When you add stuff such as this to the form builder, does it automatically become available to/from ad_form? I could wade through the code, but...

Collapse
Posted by Ola Hansson on
Jun, Roberto:

Would you mind briefly explaining what the form-builder wizard is and what it does?

Collapse
Posted by Jun Yamog on
Hi Ola,

The form builder wizard, well its not actually restricted to forms.  The wizard gives you nice good api to make a series of pages in sequence.  Normally the page contains the forms built by either ad_form or the normal template calls.

You can define the state variables that you want to keep across the steps.  The way it technical does things is this way:

step 1 is a form... it submits the form and it gets processed, if you are using ad_form on_submit may define to tell the wizard to step to the next step, which is step 2.  It then returns a redirect to the next step together with any state variables that was set on the previous steps.

Collapse
Posted by Roberto Mello on
Ola,

No, Lars' changes don't automatically become available to ad_form.

I modified ad_form so that it could take advantage of Lars' latest modifications, and also backported his mods to 4.6.

I also modified ad_form so it'd take section attributes to form elements. These were really small changes, but they work nicely. I'll be providing the patches as soon as I finish the Changelog for 4.6.

-Roberto

Collapse
Posted by Lars Pind on
Roberto,

You didn't commit your changes to ad_form yet, did you?

Because I was planning on rewriting bug-tracker to use these new features, and to use ad_form, today, so I'll be needing them.

/Lars

Collapse
Posted by Roberto Mello on
Hi Lars,

No, I had not committed my changes to ad_form yet because I wanted to make sure they were working right, and to make them available for OpenACS 4.6.

I made a tar ball with all my changes, and everything one needs to get all of this to a stock OpenACS 4.6 tree. It includes your recent additions (which rock, btw), DanW's recent modification to ad_form (set form vars from edit_request block, which is also very nice), and support for -section attributes to ad_form (full README on my web site).

I also made screenshots, ad_form'ized your display-edit.tcl/adp, modified stardand-lars.adp to use CSS and to display multiple checkboxes in a better way.

It's all documented and available for download here: http://www.brasileiro.net/code/

I will be comitting the ad_form changes in a minute.

-Roberto

Collapse
Posted by Roberto Mello on
ad_form changes are now committed to HEAD.

-Roberto

Collapse
Posted by Ola Hansson on
I'm looking at ad_form, and I see that there has been support for form sections since revision 1.1, actually, using this syntax:
ad_form -form {

    {-section "Section One"}
	{email_to:text(text)    {label "To"}    {html {size 30}} }

    {-section "Section Two"}
    {email_subject:text(text)    {label "Subject"}   {html {size 40}}
	spellcheck}
    {email_body:text(textarea)    {label "Body"}    {html {rows 10 cols 50}} }

} -on_submit {

        # send email

}
This has a little less typing to it when there are many elements in a section, but it's just a matter of taste. The oter way of specifying sections is Roberto's new way - http://brasileiro.net/code/forms/ .
Collapse
Posted by Roberto Mello on
Ola,

I missworded my statement here in the forum when I said "support for -section attributes to ad_form".

But I worded it right in my README (http://www.brasileiro.net/code/forms/README-forms.txt) in the tarball I made for 4.6:

"Added support for the templating system's -section attribute to form elements, to ad_form"

which is accurate. ad_form's 1.1 implementation of -section may be interpreted differently in the future from the element attribute -section that I added support for.

The non-element-attribute section would be (in my view) a way to generate a separation between elements. Whereas the element attribute -section could be interpreted as a way to group elements together, even if there's no visible separation row like the current standard.adp template for forms generates.

In summary, I think they serve different purposes, but maybe Lars should answer that as he can explain what was in his mind.

-Roberto

Collapse
Posted by Lars Pind on
Apologies to all, but in the process of upgrading bug-tracker to make use of the new form builder display/edit mode feature, and the new button features, I realized that I hadn't designed the quite right the first time.

So now I've committed a new version.

It's mostly the same, but:

- The form builder now handles the logic of which buttons to display, e.g. when you say -have_submit, or when you switch between display and edit modes, etc.

- There's a new "action" feature, where you can have buttons for actions on the "display" version, which cause the form to go into "edit" mode, and to pass on the action using a "form:action" hidden variable. See the latest version of bug-tracker on HEAD for an example of how this works.

/Lars

Collapse
Posted by C. R. Oldham on
So does this break anything in the old form builder?  Can we backport this to 4.6?  And are there docs for this somewhere?
Collapse
Posted by Roberto Mello on
I'm backporting it to 4.6. It'll be available first here (http://www.brasileiro.net/code/), then it'll be commited to the 4.6 branch on CVS.

-Roberto

Collapse
Posted by Ola Hansson on
FYI, I am working on integrating a spellchecking facility into the form builder/ad_form which is going to use aspell/ispell and be based on (3.x) work done by Jin Choi and Eve Anderson from aD.

I hope to have this working within a few days or so if all goes well ...

My current idea (but I'm open to other suggestions) is to add some switches to ad_form; "-spellcheck_template foo" and "-spellcheck_label {Check Spelling}" (plus a "-submit_label {Blah}" and possibly a "-confirm_label {Confirm Forum Posting}" while I'm at it - but that is unrelated to spellchecking). Even though the spellcheck_template (which takes care of all the spellchecking) in a way permitts confirmation of the submitted text(s), I think we should be able to specify a confirm_template at the same time as spellchecking is specified, and that it should jump in after you're happy with the spelling (if indicated by the script, that is).

I am envisioning it something like this:


set check [parameter::get_from_package_key -package_key acs-kernel \
               -parameter SpellcheckFormP -default true]


set all_widgets {}
lappend all_widgets \
    [list email_to:text(text) {label "To"} {html {size 30}}] \
    [list email_subject:text(text),spellcheck($check) {label "Subject"} {html {size 40}}] \
    [list email_body:text(textarea),spellcheck($check) {label "Body"} {html {rows 10 cols 50}}]


ad_form -name spell \
    -mode edit \
    -cancel_url / \
    -cancel_label "Get outta here" \
    -confirm_template confirm \
    -spellcheck_template spellcheck \
    -submit_label "Submit" \
    -form $all_widgets \
    -on_submit {

    # send email

}

From the developer/admin perspective:

Many elements can be spellchecked (I'm hoping) but not all have to be. The developer can choose only some of the text/textarea elements as s/he sees fit. The site administrator can enable/disable spellchecking on a whole site via a kernel parameter (if the developer uses the technique showed above when setting vars).

From the user perspective:

The user faces a big form - now, if the developer has specified spellchecking on one text field and on one textarea, say, and has also specified a spellcheck_template that is working, the user will not just see a single "OK" submit button but will also see a "Check Spelling" button (and maybe also a "Cancel" button which will be the button farthest to the right). When the user hits the spell button s/he is sent off to the spellcheck_template and, after eventual spelling correction, sent back again (or to the confirm_template) ...

I think it would be possible to make this feature available to the form builder (and not only to ad_form) or that it is at least something to strive towards.

If you have comments or suggestions, I sure want to hear them now 😊

/Ola

Collapse
Posted by Ola Hansson on
I should add.

If none of the elements of the form has specified that it should be "spellcheckable" no such submit button will be presented, only "OK" and maybe "Cancel".

Collapse
Posted by Ola Hansson on
Wow!

Lars, I *really* like your recent changes to the "standard-lars" template. Truly awesome...

I think it is about time we let that one be the default form template in the toolkit.

(If obligatory fields are left unfilled those entire elements gets a red border when validation fails while the other elements remain purple/background-colored - something like that)

/Ola

Collapse
Posted by Jon Griffin on
Roberto,
Did you update your tarball with the latest changes?

It seems that I am misunderstanding something with the view/edit interaction. The edit button brings me to /index. Shouldn't it change modes to edit or something like that?

Collapse
Posted by Roberto Mello on
Jon,

I haven't integrated Lars' latest changes to the templates. I will this weekend, when I'm done with my avalanche of homework.

The edit button should take you to the same form, but this time showing it with places for the user to input data.

-Roberto

Collapse
Posted by Jon Griffin on
Roberto,

Either I have a completly hosed system or your patch is missing something.

As a test I took the code off of your site and when I run it, again the edit code doesn't work. Can you please try this on an upgraded system (if possible) and let me know. Or send me a know working file set .adp,.tcl and I will try it again.