Conversation
WalkthroughThe change in src/classes/class-se-blocks.php adds a local variable $date_count = 0 within event_info_render. A new conditional branch checks when count($event_dates) == 0 and $output is empty; in that case, it retrieves se_options['past_event_notice'] and, if non-empty, appends a notice to $output as a div.wp-block-se-event-info containing p.se-event-past-notice with the notice text. Existing past-date filtering logic remains intact. No public method signatures were modified. Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/classes/class-se-blocks.php (2)
265-265: Remove unused$date_count.
$date_countis never read; triggers PHPMD UnusedLocalVariable. Delete it to keep the scope clean.- $date_count = 0;
373-381: Ensure the notice shows with a sensible default (align editor vs front‑end).In
block_assets()you default the editor setting to "Event has passed" when the option is missing, but here the front‑end only renders a notice if the option exists and is non‑empty. This creates an editor/front‑end mismatch. Also preferempty()overcount(...) === 0for safety.- // Maybe show the event passes message. - if ( 0 === count( $event_dates ) && '' === $output ) { - $options = get_option( 'se_options' ); - if ( isset( $options['past_event_notice'] ) && ! empty( $options['past_event_notice'] ) ) { - $output .= '<div class="wp-block-se-event-info">'; - $output .= '<p class="se-event-past-notice">' . esc_html( $options['past_event_notice'] ) . '</p>'; - $output .= '</div>'; - } - } + // Maybe show the event passed message. + if ( empty( $event_dates ) && '' === $output ) { + $options = get_option( 'se_options' ); + $notice = isset( $options['past_event_notice'] ) && $options['past_event_notice'] !== '' + ? $options['past_event_notice'] + : __( 'Event has passed', 'simple-events' ); + + $notice = trim( (string) $notice ); + if ( $notice !== '' ) { + $output .= '<div class="wp-block-se-event-info">'; + $output .= '<p class="se-event-past-notice">' . esc_html( $notice ) . '</p>'; + $output .= '</div>'; + } + }Verification ask:
- Confirm desired behavior when “Add to calendar links” are enabled. Since those append to
$outputearlier, the notice won’t render even if all dates are filtered out—consistent with “not rendered output,” but please verify this matches the product intent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/classes/class-se-blocks.php(3 hunks)
🧰 Additional context used
🪛 PHPMD (2.15.0)
src/classes/class-se-blocks.php
265-265: Avoid unused local variables such as '$date_count'. (undefined)
(UnusedLocalVariable)
🔇 Additional comments (1)
src/classes/class-se-blocks.php (1)
278-278: No-op whitespace.Nothing to do here.
Changes proposed in this Pull Request
Testing instructions
Mentions #
Summary by CodeRabbit