Skip to:
Content

BuddyPress.org

Opened 12 years ago

Closed 11 years ago

Last modified 9 years ago

#4974 closed enhancement (fixed)

Add filters to bp_directory_groups_search_form() and bp_directory_members_search_form()

Reported by: megainfo's profile megainfo Owned by: boonebgorges's profile boonebgorges
Milestone: 1.9 Priority: low
Severity: minor Version:
Component: Templates Keywords: has-patch needs-refresh
Cc:

Description

I suggest adding some filters for the html code returned by

bp_directory_groups_search_form() and bp_directory_members_search_form()

Attachments (5)

4974.diff (2.3 KB) - added by dimensionmedia 11 years ago.
Updates to bp-members-template and bp-groups-template
4974.2.diff (3.6 KB) - added by dimensionmedia 11 years ago.
4974.3 (1.5 KB) - added by hnla 11 years ago.
Corrects value attr in inputs
4974.4.diff (6.2 KB) - added by dimensionmedia 11 years ago.
4974.3.diff (2.6 KB) - added by dimensionmedia 11 years ago.

Download all attachments as: .zip

Change History (21)

#1 @boonebgorges
12 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 1.9
  • Priority changed from normal to low
  • Severity changed from normal to minor

I think we can do this.

#2 @dimensionmedia
11 years ago

Looked simple enough, but let me your know feedback. First time submitting a patch, but don't be gentle.

#3 @boonebgorges
11 years ago

  • Keywords has-patch needs-refresh added; needs-patch removed

Technique looks fine in bp_directory_groups_search_form(), but it looks like the implementation in bp_group_search_form() is incomplete. The ?> delimiter needs to be moved, the nonce field needs to be included in the string, and the string needs to be echoed and run through apply_filters().

@dimensionmedia
11 years ago

Updates to bp-members-template and bp-groups-template

#4 @dimensionmedia
11 years ago

hhmm... not sure what happened. Ticket wasn't even concerning bp_group_search_form(), but nevertheless I applied the same technique to that function as well.

Please note that 4972.2.diff attachement contains the latest code adjustments.

#5 @boonebgorges
11 years ago

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

In 7328:

Adds filters to search form HTML functions

Filters introduced in bp_directory_members_search_form(),
bp_directory_groups_search_form(), and bp_group_search_form() allow theme
and plugin authors more flexibility with core-generated search form HTML.

Fixes #4974

Props dimensionmedia

#6 @boonebgorges
11 years ago

Looks good. Thanks!

#7 @hnla
11 years ago

  • Keywords has-patch removed
  • Resolution fixed deleted
  • Status changed from closed to reopened

A few points and a patch correction.

Patch 4974.3 corrects escaped value attr string; needs to be passed through not echoed: () rather than _e().

1/ Why have we only highlighted two of the dir search forms for this re-factoring, is there a reason the legacy forum search and blogs search weren't changed?.

2/ Where is bp_group_search_form() actually used? I can find no reference to it other than the groups template core file

Version 0, edited 11 years ago by hnla (next)

@hnla
11 years ago

Corrects value attr in inputs

#8 @boonebgorges
11 years ago

In 7339:

Use the correct translation function when building search form HTML

This corrects incorrect use of _e() in r7328

See #4974

Props hnla

#9 @boonebgorges
11 years ago

  • Keywords needs-patch added; needs-refresh removed

Good catch on _e(), hnla. Thanks for the patch.

Your questions:

1 - We've only changed it here because this is all the original patch did. It would be nice to extend the same treatment to all search form html generating functions. We just need a patch.

2 - I don't think it is used anywhere by BP, nor has it been for a number of years. We have to keep it in there for backward compatibility, as some themes/installations may be using it.

#10 @hnla
11 years ago

1 - We've only changed it here because this is all the original patch did. It would be nice to extend the same treatment to all search form html generating functions. We just need a patch.

The other two will need patching as it could prove awkward for developers wanting to re-factor all of them.

2 - I don't think it is used anywhere by BP, nor has it been for a number of years. We have to keep it in there for backward compatibility, as some themes/installations may be using it.

Oh my favourite phrase 'backwards compatibility' - I can't find theme use of the function checking back to 1.2 locally, although perhaps not strictly 'deprecated' can this not be pulled out of the file and added to one of those /bp-core/deprecated/ files, less clutter in current core files?

#11 @dimensionmedia
11 years ago

Added filters for blog and forum search forms.

#12 @hnla
11 years ago

@dimensionmedia
Good stuff, glad I spotted the ticket update before doing so myself, however why did you re-patch the whole lot?

A final thing to consider before the patch/ticket closed: The labels are empty, they ought not to be, text ought to be added hidden if necessary with a assistive-text class - as found iirc in one of the WP default stylesheets of course this may upset themes that have overridden or copied all BP styles.

#13 @boonebgorges
11 years ago

  • Keywords has-patch needs-refresh added; needs-patch removed

dimensionmedia - Thanks for the new patch. Would you mind regenerating it against the latest trunk? The one you have here won't apply cleanly, because some of the changes have already been committed.

hnla - The label stuff is separate from the main issue discussed in this ticket. Perhaps you could open a new one and we could hash it out there.

#14 @dimensionmedia
11 years ago

Updated bp_directory_forums_search_form() and bp_directory_blogs_search_form() against latest trunk.

#15 @boonebgorges
11 years ago

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

In 7345:

Adds filters to Blogs and Forums search form markup

Fixes #4974

Props dimensionmedia

#16 @DJPaul
9 years ago

  • Component changed from Appearance - Template Parts to Templates
Note: See TracTickets for help on using tickets.