Skip to:
Content

BuddyPress.org

Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#7532 closed defect (bug) (fixed)

BP_Button needs to update `$parent_element` and `$parent_attr`

Reported by: r-a-y Owned by:
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.8.0
Component: Core Keywords: has-patch
Cc:

Description (last modified by r-a-y)

If you are filtering 'bp_get_button' and want to make adjustments to the button based on what the $parent_element or $parent_attr is, you can't since those properties are not updated in the BP_Button class.

For example:

<?php
add_filter( 'bp_get_button', function( $retval, $args, $button ) {
        // Check to see if we're using an older button.
        // This will fail at the moment.
        // You could use $button->wrapper, but then you would need to re-render
        // the button element with $button->parent_attr, which would also need
        // to be updated.
        if ( 'div' === $button->parent_element) {
                // Do something here like re-render the button.
        }
        return $retval;
}, 10, 3 );

Attachments (1)

7532.01.patch (550 bytes) - added by r-a-y 2 years ago.

Download all attachments as: .zip

Change History (8)

@r-a-y
2 years ago

#1 @r-a-y
2 years ago

  • Description modified (diff)

This ticket was mentioned in Slack in #buddypress by r-a-y. View the logs.


2 years ago

#3 @hnla
2 years ago

@r-a-y also, and not sure if you read my comments in slack, we need to be able to acknowledge in the class what those buttons parent container is element wise so we can force $parent_element to 'li' if the $container returns 'ul' this is similar to the approach I took in Nouveau arg functions where if 'ul' are set them the $parent_element must be a 'li' type. Currently plugins might not be aware of args available and if not setting $parent_element the default 'div' will be used breaking A ul construct.

#4 @r-a-y
2 years ago

I created this ticket because of your comment on Slack :)

There is no $container argument in the BP_Button class. Can you link me to the Nouveau ticket where you are doing this?

#5 @hnla
2 years ago

Hmm good point the $container arg only really presents it's self in Nouveau, bp_button class only ever deals with the actual element and direct parent.

In Nouveau we have a function handling building our action buttons after getting a return from bp_get_button()

includes/functions.php L:228 bp_nouveau_wrapper()

The template function will pass $container(or not) and in the function args we are testing the value of and setting defaults.

The container value will be in the $args we're passing to bp_get_button() just not being made use of.

#6 @r-a-y
2 years ago

  • Resolution set to fixed
  • Status changed from new to closed

Damn it! Tagged the wrong ticket number in the commit message.

See r11563.

Regarding Nouveau, will try to look into this in the next few days.

#7 @hnla
2 years ago

One thing I ommited to say but had realised when I looked at this initially was that this $args['container'] that I pass in to the Nouveau functions and check value of was never then set as e.g $buttons['accept_friendship'']['container'] which we pass through to BP_Buttons_Group

I needed to add this arg in to test & will need to update all Nouveau args so this value is always available.

Note: See TracTickets for help on using tickets.