[buddypress-trac] [BuddyPress Trac] #7226: Update BP_buttons class to accept new arg param for $element_type

[buddypress-trac]

#7226: Update BP_buttons class to accept new arg param for $element_type
-------------------------+------------------
 Reporter:  hnla         |       Owner:
     Type:  enhancement  |      Status:  new
 Priority:  normal       |   Milestone:  2.7
Component:  Core         |     Version:
 Severity:  normal       |  Resolution:
 Keywords:  has-patch    |
-------------------------+------------------

Comment (by hnla):

 @boonebgorges Yep the naming conventions is a tricky one, not seen so
 clearly in this one class are the various  and slightly longwinded args
 building that have to originate in the Nouveau calling function and filter
 down through to bp_get_button().

 I think with the button names or 'button' prefix I was wanting to be clear
 in what I was dealing with, also I think I had decided that there was not
 going to be a choice to run the element as 'input', too many choices spoil
 the developer :)

 I will re-think that, I will allow for the possibility of an `$element =
 'input'` and re-set those button var names, & hope it doesn't confuse
 things. We can't justify denying the input use it's legit and our job done
 well is to facilitate dev options and bp_get_button() then becomes a more
 useful little generic function.

 >In the documentation, you say that button_type accepts 'button' or
 'submit'. Should this be enforced? (IMO, no - it's implied that the
 developer should adhere to HTML spec. We should remove the "accepts"
 language from the parameter documentation for this reason, and just in
 case they decide to add more possible values, like 'reset'.)

 I may need some additional clarity and help on this aspect - whether we
 are simply referring to documentation or how in actuality the attr is set.
 I'll review the attr as this is a mandatory one and I'm defaulting to
 `type="button"` due to assuming  majority of the time these action type
 buttons will fall outside a form construct, however equally `type="reset"`
 is valid if not a little out of fashion but ought to be allowed for.

 >data_attr is a bit funky

 Yep just knew someone would spot that :) sheer laziness in having to split
 the attr name into two parts and going stir crazy with arg values - I did
 consider what you describe above in the nested array and will re-factor to
 that.

 I'll make these changes and push a patch up later today.

 Last thing I could do with is an eye cast over the final apply_filter()
 L:356 just to make sure that's correct.

--
Ticket URL: <https://buddypress.trac.wordpress.org/ticket/7226#comment:8>
BuddyPress Trac <http://buddypress.org/>
BuddyPress Trac