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.

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

Podcast player: Render podcast header markup by server-side #15221

Merged
merged 9 commits into from
Apr 1, 2020
55 changes: 52 additions & 3 deletions extensions/blocks/podcast-player/podcast-player.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ function register_block() {
BLOCK_NAME,
array(
'attributes' => array(
'url' => array(
'url' => array(
'type' => 'url',
),
'itemsToShow' => array(
'itemsToShow' => array(
'type' => 'integer',
'default' => 5,
),
'showCoverArt' => array(
'showCoverArt' => array(
'type' => 'boolean',
'default' => true,
),
Expand Down Expand Up @@ -131,6 +131,7 @@ function render_player( $player_data, $attributes ) {
class="<?php echo esc_attr( $player_classes_name ); ?>"
style="<?php echo esc_attr( $player_inline_style ); ?>"
>
<?php render( 'podcast-header', $player_props ); ?>
<ol class="jetpack-podcast-player__tracks">
<?php foreach ( $player_data['tracks'] as $attachment ) : ?>
<li
Expand Down Expand Up @@ -214,3 +215,51 @@ function get_colors( $name, $attrs, $property ) {

return $colors;
}

/**
* Render the given template in server-side.
* Important note:
* The $template_props array will be extracted.
* This means it will create a var for each array item.
* Keep it mind when using this param to pass
* properties to the template.
*
* @param string $name Template name, available in `./templates` folder.
* @param array $template_props Template properties. Optional.
* @param bool $print Render template. True as default.
* @return false|string HTML markup or false.
*/
function render( $name, $template_props = array(), $print = true ) {
if ( ! strpos( $name, '.php' ) ) {
$name = $name . '.php';
}

$template_path = dirname( __FILE__ ) . '/templates/' . $name;

if ( ! file_exists( $template_path ) ) {
return '';
}

// Optionally provided an assoc array of data to pass to template
// IMPORTANT: It will be extracted into variables.
if ( is_array( $template_props ) ) {
// It ignores the `discouraging` sniffer rule for extract,
// since it's needed to make the templating system works.
extract( $template_props ); // phpcs:ignore WordPress.PHP.DontExtract.extract_extract
}

ob_start();
include $template_path;
$markup = ob_get_contents();
ob_end_clean();

if ( $print ) {
// it's disabled in order to allow to templates
// render their content without HTML entities issues.
// However, each template is going to be checked
// guaranteeing the correct escape for the markup.

echo $markup; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped
}
return $markup;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
<?php
/**
* Podcast Header Title template.
*
* @package Jetpack
*/

namespace Automattic\Jetpack\Extensions\Podcast_Player;

// phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
// phpcs:disable WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase

if ( ! isset( $title ) && empty( $track ) && ! isset( $track['title'] ) ) {
return;
}
?>

<h2 id=<?php echo esc_attr( $playerId ); ?>__title" class="jetpack-podcast-player__title">
<?php if ( ! empty( $track ) && isset( $track['title'] ) ) : ?>
<span class="jetpack-podcast-player__current-track-title">
<?php echo esc_attr( $track['title'] ); ?>
</span>
<?php endif; ?>

<?php if ( ! empty( $track ) && isset( $track['title'] ) && isset( $title ) ) : ?>
<span class="jetpack-podcast-player--visually-hidden"> - </span>
<?php endif; ?>

<?php if ( isset( $title ) ) : ?>
<?php
render(
'podcast-title',
array(
'title' => $title,
'link' => $link,
)
);
?>
<?php endif; ?>
</h2>

<?php
// phpcs:enable WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
// phpcs:enable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
59 changes: 59 additions & 0 deletions extensions/blocks/podcast-player/templates/podcast-header.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
<?php
/**
* Podcast Header template.
*
* @package Jetpack
*/

namespace Automattic\Jetpack\Extensions\Podcast_Player;

// phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
// phpcs:disable WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase

/**
* Block attributes
*/
$attributes = (array) $template_props['attributes']; // phpcs:ignore VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
$show_cover_art = (bool) $attributes['showCoverArt'];
$show_episode_description = (bool) $attributes['showEpisodeDescription'];

// Current track.
$track = ! empty( $template_props['tracks'] ) ? $template_props['tracks'][0] : array();
?>

<div class="jetpack-podcast-player__header">
<div class="jetpack-podcast-player__current-track-info" aria-live="polite">
<?php if ( $show_cover_art && isset( $cover ) ) : ?>
<div class="jetpack-podcast-player__cover">
<img class="jetpack-podcast-player__cover-image" src=<?php echo esc_url( $cover ); ?>alt="" />
</div>

<?php
render(
'podcast-header-title',
array(
'playerId' => $playerId,
'title' => $title,
'link' => $link,
'track' => $track,
)
);
?>
<?php endif; ?>
</div>

<?php
if ( $show_episode_description && ! empty( $track ) && isset( $track['description'] ) ) :
?>
<div
id="<?php echo esc_attr( $playerId ); ?>__track-description"
class="jetpack-podcast-player__track-description"
>
<?php echo esc_attr( $track['description'] ); ?>
</div>
<?php endif; ?>
</div>

<?php
// phpcs:enable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
// phpcs:enable WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
35 changes: 35 additions & 0 deletions extensions/blocks/podcast-player/templates/podcast-title.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
<?php
/**
* Podcast Title template.
*
* @package Jetpack
*/

namespace Automattic\Jetpack\Extensions\Podcast_Player;

// phpcs:disable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable
// phpcs:disable WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
Comment on lines +10 to +11
Copy link
Contributor

@getdave getdave Apr 1, 2020

Choose a reason for hiding this comment

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

I wish there was a better way to do this?

It looks like we can set certain variables to be ignored from this UNdefinedVariable check.

https://github.com/sirbrillig/phpcs-variable-analysis#customization

I proposed we

  • update the var from $data to $templateProps
  • add a rule to the .phpcs.xml.dist under the validUndefinedVariableNames property to ignore the var $templateProps.
  • update the code in the render so that it doesn't perform an extract() and simply passes the $templateProps array.
  • the individual templates can then extract the data from $templateProps (or not) as required without having to worry about disabling phpcs warnings.

@jeherve what do you think about this? Having to litter each template with these disable rules feels really horrible.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that sounds like a good plan!


if ( ! isset( $title ) ) {
return;
}
?>

<?php if ( isset( $link ) ) : ?>
<a
class="jetpack-podcast-player__podcast-title"
href="<?php echo esc_url( $link ); ?>"
target="_blank"
rel="noopener noreferrer nofollow"
>
<?php echo esc_attr( $title ); ?>
</a>
<?php else : ?>
<span class="jetpack-podcast-player__podcast-title">
<?php echo esc_attr( $title ); ?>
</span>;
<?php endif; ?>

<?php
// phpcs:enable WordPress.NamingConventions.ValidVariableName.VariableNotSnakeCase
// phpcs:enable VariableAnalysis.CodeAnalysis.VariableAnalysis.UndefinedVariable