DRY up input views

[ewinslow]

There is so much repeated code -- it should not be so!

My suggestion would be to add an input/input view that can take all relevant parameters. Then refactor the others to feed into that one.

I hope you find that the patch I submitted gets you most of the way there.

[ewinslow]

This patch is against the 1.7 branch, so it might not be completely compatible with 1.8 in terms of class defaults

[cash]

I should have mentioned that I started working this week on something similar. It takes in all the parameters and puts them in an attribute[] array so that the output code looks like this:

<input 
<?php 
    foreach($attributes as $key=>$value) {
        echo "$key=\"$value\" ";
    }
?>
>

Each view sets its defaults in attributes first and then I cycle through what I was passed in. An advantage of this is that we wouldn't need to add new parameters explicitly like your suggestion of placeholder.

[ewinslow]

I like it. It also means you can do more natural things like

'onclick' => '...code...'

rather than

'js' => 'onclick="...code..."'

I suppose my suggestion (and patch) was a step in that direction, but we might as well take it all the way there!

[brettp]

Sounds like a good idea to me. As always, we'll need to be mindful of backward compatibility...

[cash]

This hasn't been DRYed up at all but it illustrates where I was headed (obviously cleaning up some of the stuff that elgg_view() throws in there would make this cleaner).

[ewinslow]

Cash, this looks great. A few comments/questions:

• Is 'js' still supported just for backward compatibility?
• I'd be very happy if elgg_view no longer dumped a ton of stuff into $vars.
• Might be good to sort the attributes alphabetically (makes gzipping more efficient).
• I'd be happy to implement this for the rest of the input views in a DRY fashion if you haven't already.
• A convenience function that unsets the default $vars parameters might be nice.

[ewinslow]

Oh, and we should do addslashes($v) to make sure that

'onclick' => 'call("me")'

outputs what it's expected to output. I.e., not this:

<input onclick="call("me")" />

[cash]

• js is there for backward compatibility. We can deprecate it in 1.8 and following the normal deprecation path remove it for 2.0.
• I definitely want to remove everything that elgg_view() current adds except for maybe user and url since they are used so often. I have a ticket open on this. If we remove this, we can pass in 'id' and 'name' instead of 'internalid'...
• Because of the small number of keys, we are unlikely to see much compression gain from sorting. Doesn't add enough local redundancy like you would get if sorting a long list of vocabulary words. Plus, for those who still look at raw source, it's nice to have it in order that it was passed - more or less.
• Good point on the add slashes.
• If you want to code this up, that would be great! As an FYI, Brett started adding my sticky form code into the views in 1.8. I'm still working through some ideas related to that - may involve pulling the sticky form code out of the input views and put into a simple form builder.

[ewinslow]

With regards to the sorting thing, I was going off this recommendation from ​Google:

Specify HTML attributes in the same order , i.e. alphabetize them. Put href first for links (since it is most common), then alphabetize the rest. For example, on Google's search results page, when HTML attributes were alphabetized, a 1.5% reduction in the size of the gzipped output resulted.

It's not much, but it's there. Perhaps it's best if we just leave it as a best practice for developers rather than enforcing it in the views.

One more question: with the kind of generality you're getting at here, we're really only one step away from providing a view that outputs an arbitrary html tag. Would there be any usefulness for such a view?

[ewinslow]

Initial patch against 1.7.1 using Cash's attribute technique

[cash]

Quick patch!

I see these views serving two purposes:

• convenience - less typing to get an input tag (not true for some views)
• uniformity - a site can use all sorts of plugins and shouldn't have to worry about the HTML of the input tags

I don't see any advantage to a generic html tag view - especially given the overhead of the view system.

[cash]

(In [svn:7334]) Refs #2143 I need ids for urls for the widget code right now

[ewinslow]

(In [svn:7354]) Refs #2143: Added elgg_format_attributes() for generating an attribute string from an associative array. DRYed up input/output url

[ewinslow]

(In [svn:7356]) Refs #2143: DRYed up input/form

[ewinslow]

(In [svn:7357]) Refs #2143: DRY up input/text

[ewinslow]

(In [svn:7358]) Refs #2143: DRY up input/email

[ewinslow]

(In [svn:7359]) Refs #2143: DRY up input/tags

[ewinslow]

(In [svn:7360]) Refs #2143: Cleaned up input/submit

[ewinslow]

(In [svn:7364]) Refs #2143: DRY up button input views (button, reset, submit). Changed core uses of button to reflect the fact that it no longer defaults to submit

[cash]

(In [svn:7391]) Refs #2143 fix for commit [svn:7354] as ampersands were being doubly encoded

[cash]

Still to be DRYed up:

• input/access
• input/(pulldown, dropdown, select or whatever we're going to call it)
• input/file
• input/hidden
• input/longtext and plaintext
• input/password
• input/radio

[ewinslow]

(In [svn:8141]) Refs #2143: DRYed up input/file

[ewinslow]

(In [svn:8142]) Refs #2143: DRYed up input/password

[ewinslow]

(In [svn:8143]) Refs #2143: DRY up input/hidden

[ewinslow]

(In [svn:8144]) Refs #2143: DRYed up input/dropdown -- probably could be better still

[ewinslow]

(In [svn:8145]) Refs #2143: DRYed up input/plaintext

[ewinslow]

(In [svn:8146]) Refs #2143: DRYed up input/longtext -- maybe I missed it, but was there a difference b/w plaintext and longtext?

[ewinslow]

(In [svn:8147]) Refs #2143: DRYed up input/access -- wow, that feels good

[ewinslow]

(In [svn:8148]) Refs #2143: General cleanup of input views

[ewinslow]

(In [svn:8149]) Fixes #2143: DRYed up input/radio -- other views still should be addressed, but not because they aren't DRY, therefore I'm calling this ticket closed