AJAX buttons using the <button> element will need JS refactoring to work

[hnla]

If we're going to use buttons then we need to move the original link href to a data-attr and adjust the JS accordingly.

This will mirror the changes I made to the activity actions function.

1/ refactor bp_nouveau_get_groups_buttons() in includes/groups/template-tags.php update $button args to check if args = button and set data attr in $button[] and move href link to it.

2/ locate and update JS to work with data attr not href link.

For the majority of our actions I did originally want the use of data attr rather than href links as the means to integrate with our js.

[hnla]

Further to this & related to the question raised in #152

I see fundamental problems here.

If we are allowing for custom element choices & incl <buttons> in those then we really need to empty the href keys and move that link & nonce data into a data attr

At the moment as an anchor it functions in a Ajax sense as we already pass a data attr to action the button.

the issue is in providing a choice we become obliged to ensure the element is correctly formed as a button i.e no href attr.

However the use of the anchor here is not necessarily wrong we use the href to re-direct back to same page while tagging a nonce check, so it is in strict terms a link.

I started to re-factor the bp_nouveau_get_groups_buttons() but am now going to back off this for the moment.

[r-a-y]

If we are allowing for custom element choices & incl

Where do we offer this choice? I do not see anything in the customizer about this.

If this is a hardcoded thing, then I would probably suggest reverting unless there is a valid reason to switch to the <button> element. Plugins that inject buttons will still use the older <a> element anyway.

[hnla]

Where do we offer this choice?

A dev choice - remember the refactoring that I did for core that you helped on, adding args to be passed to the action buttons?

unless there is a valid reason to switch

It was requested that we observe best practise in use of anchors for links and buttons for actions, I've attempted to provide that but in doing so see concerns such as stated where if args were passed in for a button, changing our default anchor use then provision in those nouveau button action functions would need some logic to check if buttons used and to empty the href key.

However yes I'm not going to proceed any further with changing these elements, but have proceeded to re-factor the action button function for groups to check if using buttons and empty the href - we will still use our defaults as we won't pass through any args but the function is readied to a degree for this happening.

[hnla]

@r-a-y Ok can you bear with me on this latest approach & offer opinion?

The action items that rely on href links & nonces are the issue when changing over to a button element.

Following the earlier outlined approach I'm re-factoring the button functions to perform the same checks for: bp_nouveau_activity_entry_buttons( array('button_element' => 'button') ) and to empty the href args and swap to an attr suitable/allowed for the button element.

However rather than try and set a data attr or secondary data attr I'm using the attr formaction this allows the button to override the form action if exists or simply act as a form action in the first place i.e action in a similar manner to a link href.

Good thing about this approach is that we should not need to modify the JS further.

I have re-factored the bp_nouveau_get_activity_entry_buttons( $args ) for this approach locally and in respect of Favs & comment actions these work as expected ( the delete link doesn't, however I note that there's an apparent break where the set id's are not being rendered, suspect that is the hook for delete js actions.)

The formaction attr is html5, and works from IE10 up and, although not checked fully, likely for majority of modern browsers - regardless not that concerned with older browsers, this is an option to be chosen or not & not forced on people & the attr is the correct way to approach the button element.

For Nouveau we can now ( if it all pans out ok) make a decision what we ship with:

bp_nouveau_activity_entry_buttons( array('button_element' => 'button') ) for buttons

or... bp_nouveau_activity_entry_buttons( ) to default to anchors.

So with this approach we can avoid hopefully having to update JS.

@mercime perhaps you could offer an opinion too, I think ultimately many of these actions will probably remain as 'a' links due to their use of hrefs but this approach does seem a good one when <button> selected.

[r-a-y]

<button formaction="">

I think this is the wrong semantic use of the formaction attribute. The formaction attribute is meant to be used to submit forms. The href links are not form-actionable.

Which leads us back to...

<a> vs <button>

If we look at GitHub for example, they do use the <a> element for buttons that have a href. However, they also use <button> for buttons that do not have a href and requires JS to finish or initiate the action.

For example, the Edit button at the top of this page uses <button>, but the New Issue button uses <a>.

Since most of our buttons resolve to a location via href, we can probably still keep using <a>. (I only see one button that could be moved over to <button> and that is the activity's Comment button.) We could make the argument that the <a href> links will never be used anyway since Nouveau is totally JS-dependent so we could use <button> if we wanted to.

If we decide to use <button>, the HTML should look like this:

<button type="button" class="xxx" data-action="the-type-of-action" data-nonce="xxx" aria-label="xxx">Label</button>

data-action might not be necessary, but it all depends on how the JS is constructed.

tl;dr:

<a> is easier since we wouldn't have to do much of anything and it isn't necessarily wrong semantically to use it. <button> would require more changes.

It depends on what we feel comfortable with.

Extracurricular reading:

• https://css-tricks.com/use-button-element/
• https://bitsofco.de/anchors-vs-buttons/
• https://marcysutton.com/links-vs-buttons-in-modern-web-applications/

[hnla]

Thanks for the feedback r-a-y

deep shame though about the formaction but thinking about it I get the point.

but takes us back to having to muddle around with js to look for the nonce.

I've re-factored the group buttons to flip flop the nonce links from href to 'formaction' the way I have formatted that allows for fairly easy changes from 'formaction' to 'data-attr' so thats not a bad thing & at the moment if we have the option to allow args to be passed and one being button rather than anchors then we do need to provide these changes on the arrays - what we end up defaulting templates to is beside the point & agree there are cases where they remain anchors and do so e.g the manage members actions.

[hnla]

I'm going to make one last change to the bp_nouveau_get_groups_buttons to remove formaction in favour of a data-bp-*-nonce there after we'll leave this, we'll use anchors mainly and whether there is time to re-factor the JS.

`bp_nouveau_get_groups_buttons will be the format to follow for the other functions unless someone has a better take on how to do this, the other functions I'll leave alone for the moment.

[hnla]

Tentatively closing this ticket as think these buttons are now all dealt with - can re-open if necessary.