Release/3.15.0-alpha.2 - Add episode list enhancements: cards layout, colors, pagination styles#888
Release/3.15.0-alpha.2 - Add episode list enhancements: cards layout, colors, pagination styles#888
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR extends the podcast-list Gutenberg block and shortcode with configurable pagination types, layout modes (list/cards), clickable behaviors, color customization, and responsive styling. It migrates CSS variables from global scope to per-instance scope and restructures template rendering to support card-based layouts. Changes
Sequence DiagramsequenceDiagram
participant Editor as Block Editor (JS)
participant Block as Block Registry
participant Handler as Podcast_List_Block Handler
participant Presenter as Episode_List_Presenter
participant Template as podcast-list.php Template
Editor->>Block: registerBlockType with new attributes<br/>(layout, paginationType, colors)
Block->>Handler: render_callback triggered with attributes
Handler->>Presenter: render(attributes)
Presenter->>Presenter: validate layout & paginationType
Presenter->>Presenter: prepare_template_args()<br/>(format colors, determine clickable behavior)
Presenter->>Presenter: paginate(pagination_type)
Presenter-->>Template: pass template args
Template->>Template: generate instance CSS variables
Template->>Template: conditional render<br/>(cards layout vs list)
Template->>Template: render pagination<br/>(simple or full mode)
Template-->>Editor: HTML output with<br/>instance-scoped styles
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
src/components/EditPodcastList.js (1)
241-247: Consider using the localized default for Button Text fallback.The fallback
'Listen Now'is a hardcoded English string. While the PHP block attribute provides a localized default, ifbuttonTextis undefined in the editor, this hardcoded fallback will display.♻️ Suggested improvement
{isCards && <PanelRow> <TextControl label={__('Button Text', 'seriously-simple-podcasting')} - value={buttonText || 'Listen Now'} + value={buttonText || __('Listen Now', 'seriously-simple-podcasting')} onChange={(buttonText) => setAttributes({buttonText})} /> </PanelRow>}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EditPodcastList.js` around lines 241 - 247, The TextControl fallback uses a hardcoded English string ('Listen Now'); change it to use the localized default instead: read the localized default provided by the block attributes (or use the i18n __() call) and use that as the fallback for value in the TextControl inside the isCards conditional — update the TextControl usage that references buttonText and setAttributes({buttonText}) so it uses something like (buttonText || localizedDefaultButtonText) or (buttonText || __('Listen Now', 'seriously-simple-podcasting')) to ensure the fallback is localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/scss/podcast-list.scss`:
- Around line 43-55: The active pagination styles (.current and
a.page-numbers:hover) hardcode white text which breaks contrast for light
user-controlled active backgrounds; change the color declarations to use a new
CSS variable (e.g. --ssp-episode-list-pagination-active-foreground) with a safe
fallback (currently `#fff`) and apply it in both .current and a.page-numbers:hover
so text color is configurable; update the stylesheet references to use
var(--ssp-episode-list-pagination-active-foreground, `#fff`) and ensure the new
variable is exposed/wired in the template/editor configuration.
- Around line 242-287: Replace deprecated grid-gap with the modern gap property
in the SCSS selectors where it's used: change grid-gap to gap in .castos-player
.player__main, .castos-player .play-progress, and .castos-player .playback
.playback__controls; then rebuild the compiled stylesheet
(assets/css/blocks/podcast-list.css) so the updated gap rules take effect.
In `@templates/blocks/podcast-list.php`:
- Around line 94-125: The full-card overlay anchor (ssp-episode-card-link,
controlled by $is_card_clickable) is blocking interactive descendants like the
player ($player, $is_player_below) and links in the excerpt
(get_the_excerpt())—either stop rendering the overlay when interactive content
is present (e.g. if $player is truthy or excerpt contains anchors) or ensure
those descendants are placed in a positioned container with higher stacking
context (give the player/excerpt wrapper a position and higher z-index than
ssp-episode-card-link); update the conditional around the overlay or wrap the
player/excerpt output (in the ssp-episode-card-body block) so they sit above the
overlay.
- Around line 39-41: The branch that appends the modifier class
'ssp-podcast-list--col-2' currently uses the condition "$columns_per_row >= 2"
which incorrectly marks 3- and 4-column layouts as two-column; change the
condition in templates/blocks/podcast-list.php so that $wrapper_class only gets
'ssp-podcast-list--col-2' when $columns_per_row == 2 (i.e., exactly two
columns), leaving other column counts to keep their correct classes.
- Around line 33-36: The current logic makes cards "title-click" mode remove the
Listen button but the only anchor is rendered inside if ( $show_title ), so when
$show_title is false the card has no navigation target; update the conditional
setup after computing $is_title_link to detect when $is_title_link && !
$show_title and in that case fallback to a real card-level link (e.g., force
$is_card_clickable = true or set $show_listen_btn = true) so the card still has
a navigation target; ensure the rendering code that expects an anchor (the block
guarded by if ( $show_title )) is adjusted or an alternative anchor is emitted
when $is_card_clickable is true.
---
Nitpick comments:
In `@src/components/EditPodcastList.js`:
- Around line 241-247: The TextControl fallback uses a hardcoded English string
('Listen Now'); change it to use the localized default instead: read the
localized default provided by the block attributes (or use the i18n __() call)
and use that as the fallback for value in the TextControl inside the isCards
conditional — update the TextControl usage that references buttonText and
setAttributes({buttonText}) so it uses something like (buttonText ||
localizedDefaultButtonText) or (buttonText || __('Listen Now',
'seriously-simple-podcasting')) to ensure the fallback is localized.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 686aa21b-359a-4796-93e8-bf4bac09449e
📒 Files selected for processing (14)
assets/css/block-editor-styles.cssassets/css/blocks/podcast-list.cssphp/classes/handlers/class-archive-page-handler.phpphp/classes/handlers/class-settings-handler.phpphp/classes/integrations/blocks/class-podcast-list-block.phpphp/classes/presenters/class-episode-list-presenter.phpphp/classes/shortcodes/class-episode-list.phpsrc/components/EditPodcastList.jssrc/index.jssrc/scss/podcast-list.scsstemplates/blocks/podcast-list.phptests/WPUnit/EpisodeListShortcodeTest.phptests/WPUnit/PodcastListBlockTest.phptests/WPUnit/SettingsHandlerTest.php
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
templates/blocks/podcast-list.php (1)
39-41:⚠️ Potential issue | 🟠 MajorOnly add
ssp-podcast-list--col-2for exactly two columns.This template is shared with the shortcode path, and
tests/WPUnit/EpisodeListShortcodeTest.phpLine 169 still treatscolumns="3"as valid input. Inlayout="cards",>= 2will tag that render as--col-2and enable the two-column compact-player overrides fromsrc/scss/podcast-list.scssLines 241-300.Based on learnings: `src/components/EditPodcastList.js` caps the block editor control at 2 columns, but that safeguard does not apply to shortcode input.Suggested fix
- if ( $columns_per_row >= 2 ) { + if ( 2 === (int) $columns_per_row ) { $wrapper_class .= ' ssp-podcast-list--col-2'; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/blocks/podcast-list.php` around lines 39 - 41, The template currently adds the CSS class 'ssp-podcast-list--col-2' whenever $columns_per_row >= 2, which wrongly applies two-column styles for >2 columns (affecting shortcode usage); update the conditional around $wrapper_class so it only appends 'ssp-podcast-list--col-2' when $columns_per_row === 2 (check the $columns_per_row variable in the podcast-list template and change the >= 2 check to a strict equality), leaving all other logic intact so three+ columns do not get the two-column compact-player overrides.
🧹 Nitpick comments (1)
templates/blocks/podcast-list.php (1)
131-132: Let the CTA label own its arrow.
$button_textis already the customizable/translatable label, but appending→here forces the glyph onto every custom string and makes RTL/localized output harder to control. Put the arrow in the default translated label or render it in CSS instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@templates/blocks/podcast-list.php` around lines 131 - 132, The template currently appends a hardcoded arrow to the CTA by echoing $button_text followed by “→”; remove the glyph from the template so the label alone is output (i.e., change the echo to only render esc_html( $button_text ) in the .ssp-listen-now-button-content span), then put the arrow into the default translated label where appropriate or add it via CSS (for example using .ssp-listen-now-button-content::after) so RTL/localized control remains with the translation/CSS rather than the template.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/scss/podcast-list.scss`:
- Around line 1-8: The committed SCSS now defines instance-scoped variables on
.ssp-podcast-list (e.g., --ssp-episode-list-title-size,
--ssp-episode-list-article-margin-bottom, --ssp-episode-list-gap,
--ssp-episode-list-cols), but the built asset
assets/css/blocks/podcast-list.min.css still contains the old :root
--ssp-podcast-list-* variables; rebuild the block CSS bundle (run the project's
CSS build task) so the compiled podcast-list.min.css reflects the new
.ssp-podcast-list variable declarations and then commit the regenerated asset.
---
Duplicate comments:
In `@templates/blocks/podcast-list.php`:
- Around line 39-41: The template currently adds the CSS class
'ssp-podcast-list--col-2' whenever $columns_per_row >= 2, which wrongly applies
two-column styles for >2 columns (affecting shortcode usage); update the
conditional around $wrapper_class so it only appends 'ssp-podcast-list--col-2'
when $columns_per_row === 2 (check the $columns_per_row variable in the
podcast-list template and change the >= 2 check to a strict equality), leaving
all other logic intact so three+ columns do not get the two-column
compact-player overrides.
---
Nitpick comments:
In `@templates/blocks/podcast-list.php`:
- Around line 131-132: The template currently appends a hardcoded arrow to the
CTA by echoing $button_text followed by “→”; remove the glyph from the template
so the label alone is output (i.e., change the echo to only render esc_html(
$button_text ) in the .ssp-listen-now-button-content span), then put the arrow
into the default translated label where appropriate or add it via CSS (for
example using .ssp-listen-now-button-content::after) so RTL/localized control
remains with the translation/CSS rather than the template.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f5f586f0-0cc8-4bc1-856d-6409bbf82a51
📒 Files selected for processing (7)
.github/workflows/run_tests.ymlseriously-simple-podcasting.phpsrc/scss/podcast-list.scsstemplates/blocks/podcast-list.phptests/WPUnit/ArchivePageTest.phptests/WPUnit/EpisodeListShortcodeTest.phptests/WPUnit/FeedControllerTest.php
✅ Files skipped from review due to trivial changes (2)
- .github/workflows/run_tests.yml
- seriously-simple-podcasting.php
Title: Add episode list enhancements: cards layout, colors, pagination styles
Summary
layout="cards"with clickable modes (button,card,title), image+title header row, full-width image support forimage_size="full", "Listen Now" button, card hover effects, compact player in 2-column cardsPanelColorSettings(theme palette aware). Text color scoped to content elements only to avoid link inheritance issuessimple(prev/next) orfull(numbered) pagination with dedicated color settings. Backward compatible — existing blocks keepsimple, new blocks default tofullvia block variationssp-el-N) prevents style collision when multiple shortcodes appear on the same pagedisplay_player=true,display_excerpt=true,display_image=false,title_size=24,image_size=mediummediumimage size via block variation; existing blocks keepfull(Gutenberg does not serialize default attribute values)Summary by CodeRabbit
New Features
Changes