Re: More thoughts on Forge->Formo conversion

Chad Kieffer <[email protected]> Sun, 28 Apr 2013 16:29:12 -0600
Newsgroups gmane.comp.web.gallery.devel
Message-ID <[email protected]>
--===============0769219641267425552==
Content-Type: multipart/alternative; boundary=Apple-Mail-2-186412195


--Apple-Mail-2-186412195
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

Hey Shad,

Nice work on combining the login routes. That simplifies things nicely.=20=


I'll always argue for keeping semantically meaningful tags over div =
tags. I don't think using divs simplifies theming all that much. Sure, =
if a themer doesn't want the default border of a fieldset they have to =
override that, but it's just a few lines of CSS. I'd also argue for the =
use of legend tags to label forms or form sections.

Fieldsets group related form elements, even on the simplest of forms. I =
believe we get accessibility points, even if it's not something users =
are clamoring for in Gallery.=20

My $.02.

- Chad

On Apr 28, 2013, at 4:03 PM, Shad Laws wrote:

> Alright, I think I have one complete example finished up - the login =
controller works!  I squashed four routes (login/ajax, login/auth_ajax, =
login/html, login/auth_html) into one (login).  Also, I made some tweaks =
to Formo so we could keep all of our labels/errors inline (as opposed to =
in external message files) and automatically stick in our CSRF token.
>=20
> Next step - work on the formatting.  It seems that Gallery's old =
template used <li> elements for each input with a <fieldset> for each =
group.  By default, Formo's templates use a <div> on everything.  Before =
I rewrite them all, I thought I'd ask: is there a reason why we should =
prefer one or the other?  Using li/fieldset seems better for backward =
compatibility, but using divs seems more flexible for theming... =
thoughts?
>=20
> Take care,
> Shad
>=20
>=20
> On 24 April 2013 18:42, Bharat Mediratta <[email protected]> wrote:
>=20
> Fantastic.  I never liked the way we had this split up before so it's =
nice to see that Formo can streamline it for us.
>=20
> I've also never been happy with our form/{add,edit}/ routing - that =
was a framework put in place before it was clear what the true need =
would be and it's only used in 9 places.  I'd be happy to see that =
routing go away and let each controller do it in a more ad hoc approach. =
 Take a look at that while you're in there and let me know what you =
think.
>=20
> -Bharat
>=20
>=20
> On Wed, Apr 24, 2013 at 1:13 AM, Shad Laws <shad-xpYdmXCiSuZWk0Htik3J/[email protected]> wrote:
> Hey everyone,
>=20
> I'm working on getting acquainted with Formo a bit more, and it seems =
like the default, expected actions of Formo is a bit different than how =
we were using Forge in Gallery 3.0.x.
>=20
> Before, we usually (but not always) had a separate controller action =
for showing a form vs. validating a form (e.g. action_edit() and =
action_save()).  Also, it was common to have a third function generate =
the form for us, so we ended handling the form in three places.
>=20
> It seems that the default, expected behavior of Formo is to do this in =
one place with one action.  It's smart enough to figure out if we have =
post data or not, and therefore if we are in "edit" or "save" mode =
(following the example).  An example:
>=20
> public function action_edit() {
>   $form =3D Formo::form(...)
>           ->....; // build form, load with default values.
>=20
>   if ($form->load()->validate()) {
>     // load() gets the values from post/files and fills the values,
>     // then validate() checks stuff and does nothing if the form isn't
>     // yet sent (nothing loaded) or adds errors if it was.
>=20
>     // Do some more stuff... then:
>     Message::success(t("Done!"));
>     // And/or:
>     $this->redirect("somewhere/else");
>   }
>  =20
>   $view =3D .....;
>   $view->form =3D $form;
>=20
>   $this->response->body($view);
> }
>=20
> I can overload a couple Formo classes to ensure that each form =
contains and validates our csrf, similar to what we did with Forge.
>=20
> Does this seem like a reasonable approach?  Is there some other reason =
why we intentionally divided these up into multiple actions before?
>=20
> Take care,
> Shad
>=20
> =
--------------------------------------------------------------------------=
----
> Try New Relic Now & We'll Send You this Cool Shirt
> New Relic is the only SaaS-based application performance monitoring =
service
> that delivers powerful full stack analytics. Optimize and monitor your
> browser, app, & servers with just a few lines of code. Try New Relic
> and get this awesome Nerd Life shirt! =
http://p.sf.net/sfu/newrelic_d2d_apr
> __[ g a l l e r y - d e v e l ]_________________________
>=20
> [ list info/archive --> http://gallery.sf.net/lists.php ]
> [ gallery info/FAQ/download --> http://gallery.sf.net ]
>=20
>=20
> =
--------------------------------------------------------------------------=
----
> Try New Relic Now & We'll Send You this Cool Shirt
> New Relic is the only SaaS-based application performance monitoring =
service=20
> that delivers powerful full stack analytics. Optimize and monitor your
> browser, app, & servers with just a few lines of code. Try New Relic
> and get this awesome Nerd Life shirt! =
http://p.sf.net/sfu/newrelic_d2d_apr__[ g a l l e r y - d e v e l =
]_________________________
>=20
> [ list info/archive --> http://gallery.sf.net/lists.php ]
> [ gallery info/FAQ/download --> http://gallery.sf.net ]


--Apple-Mail-2-186412195
Content-Transfer-Encoding: quoted-printable
Content-Type: text/html;
	charset=us-ascii

<html><head></head><body style=3D"word-wrap: break-word; =
-webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">Hey =
Shad,<div><br></div><div>Nice work on combining the login routes. That =
simplifies things nicely.&nbsp;</div><div><br></div><div>I'll always =
argue for keeping semantically meaningful tags over div tags. I don't =
think using divs simplifies theming all that much. Sure, if a themer =
doesn't want the default border of a fieldset they have to override =
that, but it's just a few lines of CSS. I'd also argue for the use of =
legend tags to label forms or form =
sections.</div><div><br></div><div>Fieldsets group related form =
elements, even on the simplest of forms. I believe we =
get&nbsp;accessibility&nbsp;points, even if it's not something users are =
clamoring for in Gallery.&nbsp;</div><div><br></div><div>My =
$.02.</div><div><br></div><div>- Chad</div><div><br><div><div>On Apr 28, =
2013, at 4:03 PM, Shad Laws wrote:</div><br =
class=3D"Apple-interchange-newline"><blockquote type=3D"cite"><div =
dir=3D"ltr">Alright, I think I have one complete example finished up - =
the login controller works! &nbsp;I squashed four routes (login/ajax, =
login/auth_ajax, login/html, login/auth_html) into one (login). =
&nbsp;Also, I made some tweaks to Formo so we could keep all of our =
labels/errors inline (as opposed to in external message files) and =
automatically stick in our CSRF token.<div>

<br></div><div style=3D"">Next step - work on the formatting. &nbsp;It =
seems that Gallery's old template used &lt;li&gt; elements for each =
input with a &lt;fieldset&gt; for each group. &nbsp;By default, Formo's =
templates use a &lt;div&gt; on everything. &nbsp;Before I rewrite them =
all, I thought I'd ask: is there a reason why we should prefer one or =
the other? &nbsp;Using li/fieldset seems better for backward =
compatibility, but using divs seems more flexible for theming... =
thoughts?</div>

<div style=3D""><br></div><div style=3D"">Take care,</div><div =
style=3D"">Shad</div></div><div class=3D"gmail_extra"><br><br><div =
class=3D"gmail_quote">On 24 April 2013 18:42, Bharat Mediratta <span =
dir=3D"ltr">&lt;<a href=3D"mailto:[email protected]" =
target=3D"_blank">[email protected]</a>&gt;</span> wrote:<br>

<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 =
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div =
dir=3D"ltr"><br><div>Fantastic. &nbsp;I never liked the way we had this =
split up before so it's nice to see that Formo can streamline it for =
us.</div>

<div><br></div><div>I've also never been happy with our form/{add,edit}/ =
routing - that was a framework put in place before it was clear what the =
true need would be and it's only used in 9 places. &nbsp;I'd be happy to =
see that routing go away and let each controller do it in a more ad hoc =
approach. &nbsp;Take a look at that while you're in there and let me =
know what you think.</div>



<div><br></div><div>-Bharat</div></div><div =
class=3D"gmail_extra"><br><br><div class=3D"gmail_quote">On Wed, Apr 24, =
2013 at 1:13 AM, Shad Laws <span dir=3D"ltr">&lt;<a =
href=3D"mailto:shad-xpYdmXCiSuZWk0Htik3J/[email protected]" =
target=3D"_blank">shad-xpYdmXCiSuZWk0Htik3J/[email protected]</a>&gt;</span> wrote:<br>



<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 =
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir=3D"ltr">Hey =
everyone,<div><br></div><div>I'm working on getting acquainted with =
Formo a bit more, and it seems like the default, expected actions of =
Formo is a bit different than how we were using Forge in Gallery =
3.0.x.</div>





<div><br></div><div>Before, we usually (but not always) had a separate =
controller action for showing a form vs. validating a form (e.g. =
action_edit() and action_save()). &nbsp;Also, it was common to have a =
third function generate the form for us, so we ended handling the form =
in three places.</div>





<div><br></div><div>It seems that the default, expected behavior of =
Formo is to do this in one place with one action. &nbsp;It's smart =
enough to figure out if we have post data or not, and therefore if we =
are in "edit" or "save" mode (following the example). &nbsp;An =
example:</div>





<div><br></div><div><font face=3D"courier new, monospace">public =
function action_edit() {</font></div><div><font face=3D"courier new, =
monospace">&nbsp; $form =3D Formo::form(...)</font></div><div><font =
face=3D"courier new, monospace">&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; =
-&gt;....; // build form, load with default values.</font></div>





<div><font face=3D"courier new, monospace"><br></font></div><div><font =
face=3D"courier new, monospace">&nbsp; if =
($form-&gt;load()-&gt;validate()) {</font></div><div><font face=3D"courier=
 new, monospace">&nbsp; &nbsp; // load() gets the values from post/files =
and fills the values,</font></div>





<div><font face=3D"courier new, monospace">&nbsp; &nbsp; // then =
validate() checks stuff and does nothing if the form =
isn't</font></div><div><font face=3D"courier new, monospace">&nbsp; =
&nbsp; // yet sent (nothing loaded) or adds errors if it =
was.</font></div>





<div><font face=3D"courier new, monospace"><br></font></div><div><font =
face=3D"courier new, monospace">&nbsp; &nbsp; // Do some more stuff... =
then:</font></div><div><font face=3D"courier new, monospace">&nbsp; =
&nbsp;&nbsp;Message::success(t("Done!"));</font></div>





<div><font face=3D"courier new, monospace">&nbsp; &nbsp; // =
And/or:</font></div><div><font face=3D"courier new, monospace">&nbsp; =
&nbsp; $this-&gt;redirect("somewhere/else");</font></div><div><font =
face=3D"courier new, monospace">&nbsp; }</font></div>





<div><font face=3D"courier new, =
monospace">&nbsp;&nbsp;</font></div><div><font face=3D"courier new, =
monospace">&nbsp; $view =3D .....;</font></div><div><font face=3D"courier =
new, monospace">&nbsp; $view-&gt;form =3D $form;</font></div>

<div><font face=3D"courier new, monospace"><br></font></div><div><font =
face=3D"courier new, monospace">&nbsp; =
$this-&gt;response-&gt;body($view);</font></div><div><font face=3D"courier=
 new, monospace">}</font></div>

<div><br></div><div>I can overload a couple Formo classes to ensure that =
each form contains and validates our csrf, similar to what we did with =
Forge.<br></div><div><br></div><div>Does this seem like a reasonable =
approach? &nbsp;Is there some other reason why we intentionally divided =
these up into multiple actions before?</div>





<div><br></div><div>Take care,</div><div>Shad</div></div>
=
<br>----------------------------------------------------------------------=
--------<br>
Try New Relic Now &amp; We'll Send You this Cool Shirt<br>
New Relic is the only SaaS-based application performance monitoring =
service<br>
that delivers powerful full stack analytics. Optimize and monitor =
your<br>
browser, app, &amp; servers with just a few lines of code. Try New =
Relic<br>
and get this awesome Nerd Life shirt! <a =
href=3D"http://p.sf.net/sfu/newrelic_d2d_apr" =
target=3D"_blank">http://p.sf.net/sfu/newrelic_d2d_apr</a><br>__[ g a l =
l e r y - d e v e l ]_________________________<br>
<br>
[ list info/archive --&gt; <a href=3D"http://gallery.sf.net/lists.php" =
target=3D"_blank">http://gallery.sf.net/lists.php</a> ]<br>
[ gallery info/FAQ/download --&gt; <a href=3D"http://gallery.sf.net/" =
target=3D"_blank">http://gallery.sf.net</a> =
]<br></blockquote></div><br></div>
</blockquote></div><br></div>
=
--------------------------------------------------------------------------=
----<br>Try New Relic Now &amp; We'll Send You this Cool Shirt<br>New =
Relic is the only SaaS-based application performance monitoring service =
<br>that delivers powerful full stack analytics. Optimize and monitor =
your<br>browser, app, &amp; servers with just a few lines of code. Try =
New Relic<br>and get this awesome Nerd Life shirt! <a =
href=3D"http://p.sf.net/sfu/newrelic_d2d_apr__[">http://p.sf.net/sfu/newre=
lic_d2d_apr__[</a> g a l l e r y - d e v e l =
]_________________________<br><br>[ list info/archive --&gt; <a =
href=3D"http://gallery.sf.net/lists.php">http://gallery.sf.net/lists.php</=
a> ]<br>[ gallery info/FAQ/download --&gt; <a =
href=3D"http://gallery.sf.net">http://gallery.sf.net</a> =
]</blockquote></div><br></div></body></html>=

--Apple-Mail-2-186412195--


--===============0769219641267425552==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

------------------------------------------------------------------------------
Try New Relic Now & We'll Send You this Cool Shirt
New Relic is the only SaaS-based application performance monitoring service 
that delivers powerful full stack analytics. Optimize and monitor your
browser, app, & servers with just a few lines of code. Try New Relic
and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_apr
--===============0769219641267425552==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

__[ g a l l e r y - d e v e l ]_________________________

[ list info/archive --> http://gallery.sf.net/lists.php ]
[ gallery info/FAQ/download --> http://gallery.sf.net ]
--===============0769219641267425552==--