Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#526 - Provide ability to override Audio Player Controls via filter. #528

Merged
merged 4 commits into from
Jul 12, 2023
Merged

#526 - Provide ability to override Audio Player Controls via filter. #528

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cb7d712
#526 - Provide ability to override Audio Player Controls via filter.
joshuaabenazer Jul 6, 2023
9f24077
Fix a PHPCS issue. Make some text changes
dkotter Jul 6, 2023
3dfa7f8
Merge branch 'develop' into feature/526-overrride-audio-player
dkotter Jul 12, 2023
a4eede4
Minor changes to docblocks
dkotter Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 48 additions & 19 deletions includes/Classifai/Providers/Azure/TextToSpeech.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,18 +604,14 @@ public function save_post_metadata( $post_id ) {
* @return string
*/
public function render_post_audio_controls( $content ) {
global $post;

if ( ! $post ) {
return $content;
}
$_post = get_post();

if ( ! in_array( $post->post_type, self::get_supported_post_types(), true ) ) {
if ( ! $_post instanceof \WP_Post ) {
return $content;
}

$is_audio_enabled = get_post_meta( $post->ID, self::SYNTHESIZE_SPEECH_KEY, true );
if ( 'no' === $is_audio_enabled ) {
if ( ! in_array( $_post->post_type, self::get_supported_post_types(), true ) ) {
return $content;
}

Expand All @@ -625,16 +621,21 @@ public function render_post_audio_controls( $content ) {
* @since 2.2.0
* @hook classifai_disable_post_to_audio_block
*
* @param {bool} $is_disabled Whether to disable the display or not. By default - false.
* @param {bool} $post_id Post ID.
* @param {bool} $is_disabled Whether to disable the display or not. By default - false.
* @param {WP_Post} $_post The Post object.
*
* @return {bool} Whether the audio block should be shown.
*/
if ( apply_filters( 'classifai_disable_post_to_audio_block', false, $post->ID ) ) {
if ( apply_filters( 'classifai_disable_post_to_audio_block', false, $_post ) ) {
return $content;
}

$is_audio_enabled = get_post_meta( $_post->ID, self::SYNTHESIZE_SPEECH_KEY, true );
if ( 'no' === $is_audio_enabled ) {
return $content;
}

$audio_attachment_id = (int) get_post_meta( $post->ID, self::AUDIO_ID_KEY, true );
$audio_attachment_id = (int) get_post_meta( $_post->ID, self::AUDIO_ID_KEY, true );

if ( ! $audio_attachment_id ) {
return $content;
Expand All @@ -646,6 +647,40 @@ public function render_post_audio_controls( $content ) {
return $content;
}

$audio_timestamp = (int) get_post_meta( $_post->ID, self::AUDIO_TIMESTAMP_KEY, true );

if ( $audio_timestamp ) {
$audio_attachment_url = add_query_arg( 'ver', filter_var( $audio_timestamp, FILTER_SANITIZE_NUMBER_INT ), $audio_attachment_url );
}

/**
* Filters the audio player markup before display.
*
* Returning a non-false value from this filter will short-circuit building
* the block markup and instead will return your custom markup prepended to
* the post_content.
*
* Note that by using this filter, the custom CSS and JS files will no longer
* be enqueued, so you'll be responsible for either loading them yourself or
* loading custom ones.
*
* @hook classifai_pre_render_post_audio_controls
* @since 2.2.3
*
* @param {bool|string} $markup Audio markup to use. Defaults to false.
* @param {string} $content Content of the current post.
* @param {WP_Post} $_post The Post object.
* @param {int} $audio_attachment_id The audio attachment ID.
* @param {string} $audio_attachment_url The URL to the audio attachment file.
*
* @return {bool|string} Custom audio block markup. Will be prepended to the post content.
*/
$markup = apply_filters( 'classifai_pre_render_post_audio_controls', false, $content, $_post, $audio_attachment_id, $audio_attachment_url );

if ( false !== $markup ) {
return (string) $markup . $content;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious for your thoughts here @joshuaabenazer. The more I think about it, the more I lean towards just returning whatever custom markup someone adds using this new filter, instead of us prepending that to the post content.

For instance, if someone wanted to add a play button to the end of their content, this wouldn't allow them to do that. Since we pass the post content into the filter (as well as the full post object) thinking maybe it's up to whoever uses this filter to ensure they are appending/prepending the post content with their custom markup rather than us hardcoding that here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkotter Yes, I originally did not have the content appended, but I felt like it might make sense after you posted the sample code on Slack.

It would also depend on how we word the filter too, since it is namedclassifai_pre_render_post_audio_controls, I felt it gives a sense that we are in charge of the audio only.

Now come to think of it we should just give the control to the developer as that might make it a little less confusing of what is expected. If you agree, then I can work on making that change and possibly change the filter nomenclature too(?)
I will be able to pick this up early next week though as I sign off early on Fridays.

Also originally I was just going to supply $content and $audio_attachment_url params to the filter since mostly we would need that only and the remaining can be derived if needed, but it is good to provide the other params since we have it already.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you agree, then I can work on making that change and possibly change the filter nomenclature too(?)
I will be able to pick this up early next week though as I sign off early on Fridays.

Yeah, this sounds good to me 👍🏻

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@joshuaabenazer Let me know if you want me to finish this off. Hoping to get a 2.2.3 release out this week and would love to include this if possible

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dkotter got sidetracked by other priorities. I may be able to get to this on Friday or early next week. That being said I don't mind you taking over to finish this off too.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So in looking at this again, I've changed my mind and going to merge this in as-is. Looking at the render_post_audio_controls method, it builds the audio markup and prepends that to the post content. So I think having this custom filter do the same thing (take whatever custom markup someone provides and then prepending it to the post content) is fine. If we get complaints or use cases that this doesn't support, happy to look at changing this in the future

}

wp_enqueue_script(
'classifai-post-audio-player-js',
CLASSIFAI_PLUGIN_URL . 'dist/post-audio-controls.js',
Expand All @@ -662,12 +697,6 @@ public function render_post_audio_controls( $content ) {
'all'
);

$audio_timestamp = (int) get_post_meta( $post->ID, self::AUDIO_TIMESTAMP_KEY, true );

if ( $audio_timestamp ) {
$audio_attachment_url = add_query_arg( 'ver', filter_var( $audio_timestamp, FILTER_SANITIZE_NUMBER_INT ), $audio_attachment_url );
}

ob_start();

?>
Expand All @@ -691,9 +720,9 @@ public function render_post_audio_controls( $content ) {
*
* @return {string} Filtered text.
*/
apply_filters( 'classifai_listen_to_this_post_text', '%s %s', $post->ID ),
apply_filters( 'classifai_listen_to_this_post_text', '%s %s', $_post->ID ),
esc_html__( 'Listen to this', 'classifai' ),
esc_html( $post->post_type )
esc_html( $_post->post_type )
);

echo wp_kses_post( $listen_to_post_text );
Expand Down