Add/featured first argument

[mikeyarce]

Fixes #1487

Changes Proposed in this Pull Request

• Add a new featured_first argument to the [jobs] shortcode
• This ensures that now regardless of the order, featured jobs will be first!

Testing Instructions

• There are a lot of combinations to test for the jobs!

[jobs orderby="featured" featured_first="true" order="desc"]
[jobs orderby="title" featured_first="true" order="desc"]
[jobs orderby="ID" featured_first="true" order="desc"]
[jobs orderby="name" featured_first="true" order="desc"]
[jobs orderby="date" featured_first="true" order="desc"]
[jobs orderby="modified" featured_first="true" order="desc"]
[jobs orderby="rand" featured_first="true" order="desc"]
[jobs orderby="rand_featured" featured_first="true" order="desc"]
[jobs orderby="featured" featured_first="true" order="asc"]
[jobs orderby="rand_featured" featured_first="true" order="asc"]
[jobs orderby="title" featured_first="true" order="asc"]
[jobs orderby="ID" featured_first="true" order="asc"]
[jobs orderby="name" featured_first="true" order="asc"]
[jobs orderby="date" featured_first="true" order="asc"]
[jobs orderby="modified" featured_first="true" order="asc"]
[jobs orderby="rand" featured_first="true" order="asc"]
[jobs orderby="rand_featured" featured_first="true" order="asc"]
[jobs orderby="featured" featured_first="true" order="asc"]

I would also make sure to try at least a few featured_first="false" to ensure that works as well.

Release Notes

• Add new shortcode parameter, "featured_first" so you can ensure featured listings always show up on top.

---

Plugin build for c3b858a
📦 Download plugin zip
▶️ Open in playground

[thedebian]

Suggested change

	if ( ! is_null( $atts['featured_first'] ) ) {
	$atts['featured_first'] = ( is_bool( $atts['featured_first'] ) && $atts['featured_first'] ) || in_array( $atts['featured_first'], [ 1, '1', 'true', 'yes' ], true );
	}

$this->string_to_bool is already called on $atts['featured_first'], which does exactly that

[mikeyarce]

Removed in d716a58

[thedebian]

Is it really what you wanted to do ?
If $atts['featured_first'] is false, then it's empty, which means $data_attributes won't be populated.
If it's what you wanted to do, then it's ok :)

[mikeyarce]

yeah no, not what I want to do. Removed in c3b858a

[thedebian]

Does $args['featured_first'] (and all added data_attributes) have to be a string ?
If we want to test it against a bool, it's way better to test it ... well ... against a bool, but maybe there's a reason for that ?

[mikeyarce]

I'm trying to also account for people who might use featured_first="0" or featured_first="1" so checking the string worked for all cases. What do you think about something like this:

if ( true === filter_var( $args['featured_first'], FILTER_VALIDATE_BOOLEAN ) && 'featured' !== $args['orderby'] && 'rand_featured' !== $args['orderby'] ) {

[thedebian]

I thought the data here were populated by https://github.com/Automattic/WP-Job-Manager/pull/2675/files#diff-a0e82a74ed6b687d4ee6efce7bb4c78d6fa157212afa751c5c40f88306c489ddR684 , hence my question about making it boolean directly instead of string, but, if it's supposed to do more than what I thought, that's ok as-is

[thedebian]

Maybe I'm missing something here, I pasted the test shortcodes, these are all supposed to be true

Only one of them is featured

[mikeyarce]

Maybe I'm missing something here, I pasted the test shortcodes, these are all supposed to be true

What shortcode specifically wasn't working for you?

[thedebian]

Maybe I'm missing something here, I pasted the test shortcodes, these are all supposed to be true

What shortcode specifically wasn't working for you?

Ok I think I forgot to npm run build, I know this might be tedious, but can it be included in test instructions when needed ? It helps a lot when reviewing a bunch of PRs

[thedebian]

After code changes (and npm build :D), worked like a charm