Implemented additional filter to change the output of the list for linked elements #228

Closed
wants to merge 2 commits into
from

Projects

None yet

4 participants

@dasllama
Contributor

We want to use different markup for our project. So we need a filter to change the output of show_linked_elements(). I've also added the $items and $params to this filter for additional logical stuff.

@GaryJones GaryJones and 2 others commented on an outdated diff Aug 29, 2016
src/inc/common/Mlp_Helpers.php
@@ -479,7 +479,7 @@ public static function show_linked_elements( $args ) {
$output .= '</ul></div>';
- return $output;
+ return apply_filters( 'show_linked_elements', $output, $items, $params );
@GaryJones
GaryJones Aug 29, 2016 Contributor

I think this filter name should have a prefix, to avoid clashes with other plugins which might have the exact same hook name.

@tyrann0us
tyrann0us Aug 29, 2016 edited

In my opinon mlp_linked_elements_html would better fit the existing filters.

@tfrommen
tfrommen Aug 30, 2016 Member

In my opinon mlp_linked_elements_html would better fit the existing filters.

That's exactly what it will be. 👍

We don't name filter hooks just like the function/method they live in, but choose more sensible names. show_linked_elements (prefixed or not) may be misinterpreted as some sort of boolean switch for some functionality.
So mlp_linked_elements_html it is.

Also, for the sake of easier debugging, the result of the filter should be cached in the $output variable, which is returned the line after that. This way, one can step through the code, and easily see the original output and the filtered one.

Lastly, the filter is lacking appropriate documentation. Please see this one as an example.

@dasllama would you like to take care of that, or do you just want this included? :)

@tfrommen tfrommen added this to the v3.0.0 milestone Aug 30, 2016
@GaryJones GaryJones commented on the diff Aug 30, 2016
src/inc/common/Mlp_Helpers.php
@@ -478,8 +478,16 @@ public static function show_linked_elements( $args ) {
}
$output .= '</ul></div>';
-
- return apply_filters( 'show_linked_elements', $output, $items, $params );
+
+ /**
+ * Filter the output of the linked elementes
@GaryJones
GaryJones Aug 30, 2016 edited Contributor

Typo - extra e, and no full stop.

No @since.

@GaryJones GaryJones commented on the diff Aug 30, 2016
src/inc/common/Mlp_Helpers.php
@@ -478,8 +478,16 @@ public static function show_linked_elements( $args ) {
}
$output .= '</ul></div>';
-
- return apply_filters( 'show_linked_elements', $output, $items, $params );
+
+ /**
+ * Filter the output of the linked elementes
+ *
+ * @param string $output the generated HTML
+ * @param array $items the language items
+ * @param array $params the parameters which came from the original call
+ */
+ $output = apply_filters( 'mlp_linked_elements_html', $output, $items, $params );
@GaryJones
GaryJones Aug 30, 2016 Contributor

No need to assign to a variable first - just return the apply_filters() call.

@GaryJones
GaryJones Aug 30, 2016 Contributor

I see - my bad :-)

@GaryJones GaryJones commented on the diff Aug 30, 2016
src/inc/common/Mlp_Helpers.php
@@ -478,8 +478,16 @@ public static function show_linked_elements( $args ) {
}
$output .= '</ul></div>';
-
- return apply_filters( 'show_linked_elements', $output, $items, $params );
+
+ /**
+ * Filter the output of the linked elementes
+ *
+ * @param string $output the generated HTML
@GaryJones
GaryJones Aug 30, 2016 Contributor

Redundant spaces after @params.
Descriptions should start with capitals and end with full stops.

@tfrommen tfrommen modified the milestone: v2.5.0, v3.0.0 Dec 23, 2016
@tfrommen
Member

Closed with b03cd6f.

@tfrommen tfrommen closed this Dec 23, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment