Beta 2 bugfixes #445
Beta 2 bugfixes #445
Conversation
WalkthroughThe changes in this pull request introduce multiple new block variations for the "core/group" block in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend
participant Accommodation
participant CustomJS
User->>Frontend: Request accommodation page
Frontend->>Accommodation: Fetch breadcrumb links
Accommodation->>Frontend: Return breadcrumb links
Frontend->>User: Display page with breadcrumbs
User->>CustomJS: Click "Read More"
CustomJS->>CustomJS: Update button text to "Read Less"
CustomJS->>User: Display expanded content
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
WalkthroughThis update brings enhancements to the Special Interests section's styling, improves the dynamic handling of the 'Read More' button text, and introduces safer function calls. It also refines breadcrumb generation by utilizing primary terms for accommodation and travel styles, leading to a more accurate and user-friendly navigation experience. Changes
Uplevel your code reviews with CodeRabbit ProCodeRabbit ProIf you like this project, please support us by purchasing the Pro version. The Pro version has advanced context, superior noise reduction and several proprietary improvements compared to the open source version. Moreover, CodeRabbit Pro is free for open source projects. |
![codefactor-io[bot]](https://avatars.githubusercontent.com/in/25603?s=60&v=4){kind=small}
| @@ -424,41 +424,34 @@ | |||
| 'url' => get_post_type_archive_link( 'accommodation' ), | |||
| ), | |||
| ); | |||
| $current_destinations = get_post_meta( get_the_ID(), 'destination_to_accommodation', true ); | |||
|
|
|||
There was a problem hiding this comment.
Whitespace found at end of line.
| foreach ( $current_destinations as $current_destination ) { | ||
| $all_destinations[] = get_post( $current_destination ); | ||
| } | ||
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { |
There was a problem hiding this comment.
Expected 0 spaces after opening bracket; 1 found.
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | |
| if (! is_wp_error( $primary_term ) && null !== $primary_term) { |
| } else { | ||
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { |
There was a problem hiding this comment.
Expected 0 spaces before closing bracket; 1 found.
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | |
| if (! is_wp_error( $terms ) && ! empty( $terms )) { |
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { |
There was a problem hiding this comment.
Expected 0 spaces after opening bracket; 1 found.
| foreach ( $terms as $term ) { | |
| foreach ($terms as $term) { |
| $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { | ||
| if ( 0 < $counter ) { |
There was a problem hiding this comment.
Expected 0 spaces before closing bracket; 1 found.
| if ( 0 < $counter ) { | |
| if (0 < $counter) { |
| $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_travel-style', true ); | ||
| $primary_term = get_term( $primary, 'travel-style' ); | ||
|
|
||
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { |
There was a problem hiding this comment.
Expected 0 spaces after opening bracket; 1 found.
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | |
| if (! is_wp_error( $primary_term ) && null !== $primary_term) { |
| } else { | ||
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'travel-style' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { |
There was a problem hiding this comment.
Expected 0 spaces before closing bracket; 1 found.
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | |
| if (! is_wp_error( $terms ) && ! empty( $terms )) { |
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'travel-style' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { |
There was a problem hiding this comment.
Expected 0 spaces before closing bracket; 1 found.
| foreach ( $terms as $term ) { | |
| foreach ($terms as $term) { |
| $terms = wp_get_object_terms( get_the_ID(), 'travel-style' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { | ||
| if ( 0 < $counter ) { |
There was a problem hiding this comment.
Expected 0 spaces after opening bracket; 1 found.
| if ( 0 < $counter ) { | |
| if (0 < $counter) { |
| } | ||
|
|
There was a problem hiding this comment.
Whitespace found at end of line.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (4)
- assets/js/blocks/accommodation.js (1)
- assets/js/src/custom.js (3)
- includes/classes/blocks/class-templates.php (1)
- includes/classes/legacy/class-frontend.php (2)
Review comments generated (7)
- Review: 5
- LGTM: 2
Tips
Chat with 
CodeRabbit Bot (@coderabbitai)
- Reply on review comments left by this bot to ask follow-up questions. A review comment is a comment on a diff or a file.
- Invite the bot into a review comment chain by tagging
@coderabbitaiin a reply.
Code suggestions
- The bot may make code suggestions, but please review them carefully before committing since the line number ranges may be misaligned.
- You can edit the comment made by the bot and manually tweak the suggestion if it is slightly off.
Pausing incremental reviews
- Add
@coderabbitai: ignoreanywhere in the PR description to pause further reviews from the bot.
| @@ -93,6 +97,7 @@ if ( window.location.hash ) { | |||
| $(this).show(); | |||
| }); | |||
| button.addClass( 'less-link' ); | |||
| button.text( 'Read Less' ); | |||
There was a problem hiding this comment.
The hardcoded text 'Read Less' might cause issues with internationalization. It's better to use a variable that can be translated.
- button.text( 'Read Less' );
+ button.text( lsx_to.readLessText );Remember to define lsx_to.readLessText in the same way you defined lsx_to.readMoreText.
|
|
||
| // Get the primary travel style | ||
| $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_accommodation-type', true ); | ||
| $primary_term = get_term( $primary, 'accommodation-type' ); | ||
|
|
||
| $all_destinations = array(); | ||
| if ( false !== $current_destinations && ! empty( $current_destinations ) ) { | ||
|
|
||
| $country = false; | ||
| $regions = array(); | ||
|
|
||
| foreach ( $current_destinations as $current_destination ) { | ||
| $all_destinations[] = get_post( $current_destination ); | ||
| } | ||
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => $primary_term->name, | ||
| 'url' => get_term_link( $primary_term, 'accommodation-type' ), | ||
| ); | ||
| } else { | ||
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { | ||
| if ( 0 < $counter ) { | ||
| continue; | ||
| } | ||
|
|
||
| //Find the country | ||
| foreach ( $all_destinations as $destination_index => $destination ) { | ||
| if ( ( 0 === $destination->post_parent || '0' === $destination->post_parent ) && 'publish' === $destination->post_status ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => $destination->post_title, | ||
| 'url' => get_permalink( $destination->ID ), | ||
| 'text' => $term->name, | ||
| 'url' => get_term_link( $term ), | ||
| ); | ||
| unset( $all_destinations[ $destination_index ] ); | ||
| } | ||
| } | ||
|
|
||
| //Find the region | ||
| if ( ! empty( $all_destinations ) ) { | ||
| foreach ( $all_destinations as $destination_index => $destination ) { | ||
| if ( 'publish' === $destination->post_status ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => $destination->post_title, | ||
| 'url' => get_permalink( $destination->ID ), | ||
| ); | ||
| } | ||
| $counter++; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The logic for fetching the primary term and falling back to the first term if the primary is not available seems correct. However, there's a potential issue with the continue statement in the loop at line 442. If the counter is greater than 0, it will skip the current iteration but won't stop the loop. This means that the loop will continue iterating over all terms even after finding one. To improve performance, consider breaking the loop once a term has been found.
- if ( 0 < $counter ) {
- continue;
- }
+ if ( 0 < $counter ) {
+ break;
+ }| 'text' => esc_attr__( 'Tours', 'tour-operator' ), | ||
| 'url' => get_post_type_archive_link( 'tour' ), | ||
| ), | ||
| ); | ||
| $region = get_post_meta( get_the_ID(), 'departs_from', true ); | ||
| if ( false !== $region && isset( $region[0] ) ) { | ||
| $country = wp_get_post_parent_id( $region[0] ); | ||
| if ( false !== $country && '' !== $country ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => get_the_title( $country ), | ||
| 'url' => get_permalink( $country ), | ||
| ); | ||
| } | ||
|
|
||
| // Get the primary travel style | ||
| $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_travel-style', true ); | ||
| $primary_term = get_term( $primary, 'travel-style' ); | ||
|
|
||
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => get_the_title( $region[0] ), | ||
| 'url' => get_permalink( $region[0] ), | ||
| 'text' => $primary_term->name, | ||
| 'url' => get_term_link( $primary_term, 'travel-style' ), | ||
| ); | ||
| } else { | ||
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'travel-style' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { | ||
| if ( 0 < $counter ) { | ||
| continue; | ||
| } | ||
|
|
||
| $new_crumbs[] = array( | ||
| 'text' => $term->name, | ||
| 'url' => get_term_link( $term ), | ||
| ); | ||
| $counter++; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Similar to the previous comment, the logic for fetching the primary term and falling back to the first term if the primary is not available seems correct. However, the same issue with the continue statement exists here as well. Consider replacing continue with break to stop the loop once a term has been found.
- if ( 0 < $counter ) {
- continue;
- }
+ if ( 0 < $counter ) {
+ break;
+ }| if ( function_exists( 'register_block_template' ) ) { | ||
| register_block_template( 'lsx-tour-operator//' . $key, $args ); | ||
| } |
There was a problem hiding this comment.
The addition of the function_exists check before calling register_block_template is a good practice to prevent potential errors if the function is not available. This is especially important when dealing with optional or plugin-dependent functions.
+ if ( function_exists( 'register_block_template' ) ) {
+ register_block_template( 'lsx-tour-operator//' . $key, $args );
+ }This change improves the robustness of the code by ensuring that it doesn't break in environments where the register_block_template function might not be defined.
| @@ -946,7 +946,7 @@ wp.domReady(() => { | |||
| metadata: { | |||
| name: 'Special Interests', | |||
| }, | |||
| className: 'lsx-special-interests-wrapper', | |||
| className: 'lsx-special-interest-wrapper', | |||
There was a problem hiding this comment.
The class name has been changed from 'lsx-special-interests-wrapper' to 'lsx-special-interest-wrapper'. Ensure that this change is reflected in the corresponding CSS file(s) and any JavaScript that may be targeting this class. If not, it could lead to styling issues or broken functionality.
- className: 'lsx-special-interests-wrapper',
+ className: 'lsx-special-interest-wrapper',
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (9)
includes/classes/blocks/class-templates.php (2)
113-113: Fix spacing in the if statementThere are extra spaces after the opening bracket and before the closing bracket of the if statement. While this doesn't affect functionality, it's good to maintain consistent spacing.
Apply this diff to fix the spacing:
- if ( function_exists( 'register_block_template' ) ) { + if (function_exists('register_block_template')) {🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
112-115: Consider version-specific template handlingSince you're already checking for function existence, you might want to consider implementing version-specific template handling. This could include:
- Fallback templates for older WordPress versions
- Different template structures based on WordPress version
- Logging or notifying admins when modern block templates aren't supported
This would enhance backwards compatibility and provide a better experience across different WordPress versions.
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])assets/js/src/custom.js (1)
51-53: Add defensive programming for text extractionHowzit! The text node extraction looks good, but let's make it more robust by adding a default value and null check.
- lsx_to.readMoreText = $(this).contents().filter(function() { - return this.nodeType === Node.TEXT_NODE; - }).text(); + lsx_to.readMoreText = $(this).contents().filter(function() { + return this.nodeType === Node.TEXT_NODE; + }).text() || 'Read More';includes/classes/legacy/class-frontend.php (2)
428-428: Fix inline comment formattingThe inline comments are missing full stops.
Apply this change:
-// Get the primary travel style +// Get the primary travel style.Also applies to: 481-481
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 428-428: includes/classes/legacy/class-frontend.php#L428
Inline comments must end in full-stops, exclamation marks, or question marks. (Squiz.Commenting.InlineComment)
427-427: Remove trailing whitespaceThere are multiple instances of trailing whitespace in the file.
Remove trailing whitespace from lines 427, 454, and 507.
Also applies to: 454-454, 507-507
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 427-427: includes/classes/legacy/class-frontend.php#L427
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])assets/js/blocks/accommodation.js (4)
Line range hint
571-571: Fix the block name typo.There's a typo in the block name. It starts with 'llsx' instead of 'lsx'.
Apply this diff to fix the typo:
- name: 'llsx-tour-operator/suggested-visitor-types', + name: 'lsx-tour-operator/suggested-visitor-types',
Line range hint
147-147: Consider using WordPress media IDs instead of hardcoded URLs.The blocks are using hardcoded URLs for icons. This approach:
- Makes the code less maintainable
- Could break if the domain changes
- Doesn't leverage WordPress's built-in image optimization
Consider using WordPress media IDs and the
wp.data.select('core').getMedia()selector to fetch image URLs dynamically. This would also allow the images to be served from the correct domain automatically.Example implementation:
const mediaId = 12345; // Replace with actual media ID const imageUrl = wp.data.select('core').getMedia(mediaId)?.source_url;Also applies to: 246-246, 344-344, 442-442, 540-540, 638-638, 736-736, 834-834, 932-932, 1030-1030, 1128-1128
Line range hint
147-1128: Add data sanitization for post meta and connection data.The blocks are rendering post meta and connection data directly. While WordPress handles basic sanitization, consider adding explicit sanitization for enhanced security.
Consider using WordPress's sanitization functions:
// Add a sanitization function const sanitizeContent = (content) => { return wp.escapeHtml(content); }; // Use it in the metadata bindings metadata: { bindings: { content: { source: 'lsx/post-meta', args: { key: 'your_key', sanitize: sanitizeContent } } } }
Line range hint
1-1128: Optimize repeated styles using shared configurations.There's significant duplication in spacing styles across blocks. Consider extracting these into shared configurations.
Example implementation:
// Define shared styles const SHARED_STYLES = { spacing: { blockGap: '5px', padding: { top: '2px', bottom: '2px' } } }; // Use in block registration attributes: { style: SHARED_STYLES }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
assets/js/blocks/accommodation.js(1 hunks)assets/js/src/custom.js(3 hunks)includes/classes/blocks/class-templates.php(1 hunks)includes/classes/legacy/class-frontend.php(2 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeFactor
includes/classes/blocks/class-templates.php
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
includes/classes/legacy/class-frontend.php
[warning] 454-454: includes/classes/legacy/class-frontend.php#L454
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])
[warning] 440-440: includes/classes/legacy/class-frontend.php#L440
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[notice] 439-439: includes/classes/legacy/class-frontend.php#L439
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space. (Generic.Formatting.MultipleStatementAlignment-[fixable])
[warning] 442-442: includes/classes/legacy/class-frontend.php#L442
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[notice] 450-450: includes/classes/legacy/class-frontend.php#L450
Stand-alone post-increment statement found. Use pre-increment instead: ++$counter. (Universal.Operators.DisallowStandalonePostIncrementDecrement-[fixable])
[warning] 428-428: includes/classes/legacy/class-frontend.php#L428
Inline comments must end in full-stops, exclamation marks, or question marks. (Squiz.Commenting.InlineComment)
[warning] 442-442: includes/classes/legacy/class-frontend.php#L442
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 427-427: includes/classes/legacy/class-frontend.php#L427
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])
[warning] 441-441: includes/classes/legacy/class-frontend.php#L441
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 432-432: includes/classes/legacy/class-frontend.php#L432
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 441-441: includes/classes/legacy/class-frontend.php#L441
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 440-440: includes/classes/legacy/class-frontend.php#L440
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 432-432: includes/classes/legacy/class-frontend.php#L432
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 495-495: includes/classes/legacy/class-frontend.php#L495
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 481-481: includes/classes/legacy/class-frontend.php#L481
Inline comments must end in full-stops, exclamation marks, or question marks. (Squiz.Commenting.InlineComment)
[warning] 485-485: includes/classes/legacy/class-frontend.php#L485
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 485-485: includes/classes/legacy/class-frontend.php#L485
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 495-495: includes/classes/legacy/class-frontend.php#L495
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 507-507: includes/classes/legacy/class-frontend.php#L507
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])
[notice] 503-503: includes/classes/legacy/class-frontend.php#L503
Stand-alone post-increment statement found. Use pre-increment instead: ++$counter. (Universal.Operators.DisallowStandalonePostIncrementDecrement-[fixable])
[warning] 493-493: includes/classes/legacy/class-frontend.php#L493
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 494-494: includes/classes/legacy/class-frontend.php#L494
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[notice] 492-492: includes/classes/legacy/class-frontend.php#L492
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space. (Generic.Formatting.MultipleStatementAlignment-[fixable])
[warning] 493-493: includes/classes/legacy/class-frontend.php#L493
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 494-494: includes/classes/legacy/class-frontend.php#L494
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
🔇 Additional comments (3)
includes/classes/blocks/class-templates.php (1)
112-115: Excellent defensive programming practice!
The addition of the conditional check for register_block_template function is a brilliant safeguard. This prevents potential runtime errors in environments where the function might not be available, making the plugin more robust and backwards compatible.
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 113-113: includes/classes/blocks/class-templates.php#L113
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
assets/js/src/custom.js (1)
89-89: Lekker implementation of the collapsed state text
The code correctly uses the stored text for the collapsed state.
assets/js/blocks/accommodation.js (1)
949-949: LGTM: Special interests wrapper class name fixed.
The className has been correctly updated from 'lsx-special-interests-wrapper' to 'lsx-special-interest-wrapper' as per the PR objectives.
| @@ -93,6 +97,7 @@ if ( window.location.hash ) { | |||
| $(this).show(); | |||
| }); | |||
| button.addClass( 'less-link' ); | |||
| button.text( 'Read Less' ); | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make "Read Less" text configurable for translations
Eish! The "Read Less" text is hardcoded. Let's make it configurable to support different languages.
- button.text( 'Read Less' );
+ button.text( lsx_to_params.readLessText || 'Read Less' );Remember to add the readLessText parameter to your localization script:
wp_localize_script('lsx-to-script', 'lsx_to_params', array(
'readLessText' => __('Read Less', 'tour-operator'),
// ... other params
));| 'text' => esc_attr__( 'Tours', 'tour-operator' ), | ||
| 'url' => get_post_type_archive_link( 'tour' ), | ||
| ), | ||
| ); | ||
| $region = get_post_meta( get_the_ID(), 'departs_from', true ); | ||
| if ( false !== $region && isset( $region[0] ) ) { | ||
| $country = wp_get_post_parent_id( $region[0] ); | ||
| if ( false !== $country && '' !== $country ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => get_the_title( $country ), | ||
| 'url' => get_permalink( $country ), | ||
| ); | ||
| } | ||
|
|
||
| // Get the primary travel style | ||
| $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_travel-style', true ); | ||
| $primary_term = get_term( $primary, 'travel-style' ); | ||
|
|
||
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => get_the_title( $region[0] ), | ||
| 'url' => get_permalink( $region[0] ), | ||
| 'text' => $primary_term->name, | ||
| 'url' => get_term_link( $primary_term, 'travel-style' ), | ||
| ); | ||
| } else { | ||
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'travel-style' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { | ||
| if ( 0 < $counter ) { | ||
| continue; | ||
| } | ||
|
|
||
| $new_crumbs[] = array( | ||
| 'text' => $term->name, | ||
| 'url' => get_term_link( $term ), | ||
| ); | ||
| $counter++; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| $new_crumbs[] = array( | ||
| 'text' => get_the_title(), | ||
| 'url' => get_permalink(), | ||
| ); | ||
| $crumbs = $new_crumbs; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consolidate duplicate breadcrumb logic
The tour breadcrumb logic is nearly identical to the accommodation breadcrumb logic, which violates the DRY principle.
Consider applying these changes:
public function tour_breadcrumb_links( $crumbs ) {
$new_crumbs = array(
array(
'text' => esc_attr__( 'Home', 'tour-operator' ),
'url' => home_url(),
),
array(
'text' => esc_attr__( 'Tours', 'tour-operator' ),
'url' => get_post_type_archive_link( 'tour' ),
),
);
- // Get the primary travel style
- $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_travel-style', true );
- $primary_term = get_term( $primary, 'travel-style' );
-
- if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) {
- $new_crumbs[] = array(
- 'text' => $primary_term->name,
- 'url' => get_term_link( $primary_term, 'travel-style' ),
- );
- } else {
- $counter = 0;
- $terms = wp_get_object_terms( get_the_ID(), 'travel-style' );
- if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) {
- foreach ( $terms as $term ) {
- if ( 0 < $counter ) {
- continue;
- }
- $new_crumbs[] = array(
- 'text' => $term->name,
- 'url' => get_term_link( $term ),
- );
- $counter++;
- }
- }
- }
+ $travel_style = $this->get_primary_term(
+ get_the_ID(),
+ '_yoast_wpseo_primary_travel-style',
+ 'travel-style'
+ );
+
+ if ( $travel_style ) {
+ $new_crumbs[] = array(
+ 'text' => $travel_style->name,
+ 'url' => get_term_link( $travel_style, 'travel-style' ),
+ );
+ }
$new_crumbs[] = array(
'text' => get_the_title(),
'url' => get_permalink(),
);
$crumbs = $new_crumbs;
return $crumbs;
}Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 495-495: includes/classes/legacy/class-frontend.php#L495
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 481-481: includes/classes/legacy/class-frontend.php#L481
Inline comments must end in full-stops, exclamation marks, or question marks. (Squiz.Commenting.InlineComment)
[warning] 485-485: includes/classes/legacy/class-frontend.php#L485
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 485-485: includes/classes/legacy/class-frontend.php#L485
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 495-495: includes/classes/legacy/class-frontend.php#L495
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 507-507: includes/classes/legacy/class-frontend.php#L507
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])
[notice] 503-503: includes/classes/legacy/class-frontend.php#L503
Stand-alone post-increment statement found. Use pre-increment instead: ++$counter. (Universal.Operators.DisallowStandalonePostIncrementDecrement-[fixable])
[warning] 493-493: includes/classes/legacy/class-frontend.php#L493
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 494-494: includes/classes/legacy/class-frontend.php#L494
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[notice] 492-492: includes/classes/legacy/class-frontend.php#L492
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space. (Generic.Formatting.MultipleStatementAlignment-[fixable])
[warning] 493-493: includes/classes/legacy/class-frontend.php#L493
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 494-494: includes/classes/legacy/class-frontend.php#L494
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
|
|
||
| // Get the primary travel style | ||
| $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_accommodation-type', true ); | ||
| $primary_term = get_term( $primary, 'accommodation-type' ); | ||
|
|
||
| $all_destinations = array(); | ||
| if ( false !== $current_destinations && ! empty( $current_destinations ) ) { | ||
|
|
||
| $country = false; | ||
| $regions = array(); | ||
|
|
||
| foreach ( $current_destinations as $current_destination ) { | ||
| $all_destinations[] = get_post( $current_destination ); | ||
| } | ||
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => $primary_term->name, | ||
| 'url' => get_term_link( $primary_term, 'accommodation-type' ), | ||
| ); | ||
| } else { | ||
| $counter = 0; | ||
| $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' ); | ||
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | ||
| foreach ( $terms as $term ) { | ||
| if ( 0 < $counter ) { | ||
| continue; | ||
| } | ||
|
|
||
| //Find the country | ||
| foreach ( $all_destinations as $destination_index => $destination ) { | ||
| if ( ( 0 === $destination->post_parent || '0' === $destination->post_parent ) && 'publish' === $destination->post_status ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => $destination->post_title, | ||
| 'url' => get_permalink( $destination->ID ), | ||
| 'text' => $term->name, | ||
| 'url' => get_term_link( $term ), | ||
| ); | ||
| unset( $all_destinations[ $destination_index ] ); | ||
| } | ||
| } | ||
|
|
||
| //Find the region | ||
| if ( ! empty( $all_destinations ) ) { | ||
| foreach ( $all_destinations as $destination_index => $destination ) { | ||
| if ( 'publish' === $destination->post_status ) { | ||
| $new_crumbs[] = array( | ||
| 'text' => $destination->post_title, | ||
| 'url' => get_permalink( $destination->ID ), | ||
| ); | ||
| } | ||
| $counter++; | ||
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor accommodation breadcrumb logic for better maintainability
The accommodation breadcrumb logic could be improved for better maintainability and readability.
Consider applying these changes:
public function accommodation_breadcrumb_links( $crumbs ) {
$new_crumbs = array(
array(
'text' => esc_attr__( 'Home', 'tour-operator' ),
'url' => home_url(),
),
array(
'text' => esc_attr__( 'Accommodation', 'tour-operator' ),
'url' => get_post_type_archive_link( 'accommodation' ),
),
);
- // Get the primary travel style
- $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_accommodation-type', true );
- $primary_term = get_term( $primary, 'accommodation-type' );
-
- if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) {
- $new_crumbs[] = array(
- 'text' => $primary_term->name,
- 'url' => get_term_link( $primary_term, 'accommodation-type' ),
- );
- } else {
- $counter = 0;
- $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' );
- if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) {
- foreach ( $terms as $term ) {
- if ( 0 < $counter ) {
- continue;
- }
- $new_crumbs[] = array(
- 'text' => $term->name,
- 'url' => get_term_link( $term ),
- );
- $counter++;
- }
- }
- }
+ $accommodation_type = $this->get_primary_term(
+ get_the_ID(),
+ '_yoast_wpseo_primary_accommodation-type',
+ 'accommodation-type'
+ );
+
+ if ( $accommodation_type ) {
+ $new_crumbs[] = array(
+ 'text' => $accommodation_type->name,
+ 'url' => get_term_link( $accommodation_type, 'accommodation-type' ),
+ );
+ }
$new_crumbs[] = array(
'text' => get_the_title(),
'url' => get_permalink(),
);
$crumbs = $new_crumbs;
return $crumbs;
}
+/**
+ * Gets the primary term for a post, falling back to the first term if no primary term is set.
+ *
+ * @param int $post_id The post ID.
+ * @param string $meta_key The meta key for the primary term.
+ * @param string $taxonomy The taxonomy name.
+ * @return WP_Term|null The term object or null if not found.
+ */
+private function get_primary_term( $post_id, $meta_key, $taxonomy ) {
+ $primary_term_id = get_post_meta( $post_id, $meta_key, true );
+ $primary_term = get_term( $primary_term_id, $taxonomy );
+
+ if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) {
+ return $primary_term;
+ }
+
+ $terms = wp_get_object_terms( $post_id, $taxonomy );
+ if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) {
+ return $terms[0];
+ }
+
+ return null;
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Get the primary travel style | |
| $primary = get_post_meta( get_the_ID(), '_yoast_wpseo_primary_accommodation-type', true ); | |
| $primary_term = get_term( $primary, 'accommodation-type' ); | |
| $all_destinations = array(); | |
| if ( false !== $current_destinations && ! empty( $current_destinations ) ) { | |
| $country = false; | |
| $regions = array(); | |
| foreach ( $current_destinations as $current_destination ) { | |
| $all_destinations[] = get_post( $current_destination ); | |
| } | |
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | |
| $new_crumbs[] = array( | |
| 'text' => $primary_term->name, | |
| 'url' => get_term_link( $primary_term, 'accommodation-type' ), | |
| ); | |
| } else { | |
| $counter = 0; | |
| $terms = wp_get_object_terms( get_the_ID(), 'accommodation-type' ); | |
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | |
| foreach ( $terms as $term ) { | |
| if ( 0 < $counter ) { | |
| continue; | |
| } | |
| //Find the country | |
| foreach ( $all_destinations as $destination_index => $destination ) { | |
| if ( ( 0 === $destination->post_parent || '0' === $destination->post_parent ) && 'publish' === $destination->post_status ) { | |
| $new_crumbs[] = array( | |
| 'text' => $destination->post_title, | |
| 'url' => get_permalink( $destination->ID ), | |
| 'text' => $term->name, | |
| 'url' => get_term_link( $term ), | |
| ); | |
| unset( $all_destinations[ $destination_index ] ); | |
| } | |
| } | |
| //Find the region | |
| if ( ! empty( $all_destinations ) ) { | |
| foreach ( $all_destinations as $destination_index => $destination ) { | |
| if ( 'publish' === $destination->post_status ) { | |
| $new_crumbs[] = array( | |
| 'text' => $destination->post_title, | |
| 'url' => get_permalink( $destination->ID ), | |
| ); | |
| } | |
| $counter++; | |
| } | |
| } | |
| } | |
| $accommodation_type = $this->get_primary_term( | |
| get_the_ID(), | |
| '_yoast_wpseo_primary_accommodation-type', | |
| 'accommodation-type' | |
| ); | |
| if ( $accommodation_type ) { | |
| $new_crumbs[] = array( | |
| 'text' => $accommodation_type->name, | |
| 'url' => get_term_link( $accommodation_type, 'accommodation-type' ), | |
| ); | |
| } | |
| /** | |
| * Gets the primary term for a post, falling back to the first term if no primary term is set. | |
| * | |
| * @param int $post_id The post ID. | |
| * @param string $meta_key The meta key for the primary term. | |
| * @param string $taxonomy The taxonomy name. | |
| * @return WP_Term|null The term object or null if not found. | |
| */ | |
| private function get_primary_term( $post_id, $meta_key, $taxonomy ) { | |
| $primary_term_id = get_post_meta( $post_id, $meta_key, true ); | |
| $primary_term = get_term( $primary_term_id, $taxonomy ); | |
| if ( ! is_wp_error( $primary_term ) && null !== $primary_term ) { | |
| return $primary_term; | |
| } | |
| $terms = wp_get_object_terms( $post_id, $taxonomy ); | |
| if ( ! is_wp_error( $terms ) && ! empty( $terms ) ) { | |
| return $terms[0]; | |
| } | |
| return null; | |
| } |
🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 454-454: includes/classes/legacy/class-frontend.php#L454
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])
[warning] 440-440: includes/classes/legacy/class-frontend.php#L440
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[notice] 439-439: includes/classes/legacy/class-frontend.php#L439
Equals sign not aligned with surrounding assignments; expected 3 spaces but found 1 space. (Generic.Formatting.MultipleStatementAlignment-[fixable])
[warning] 442-442: includes/classes/legacy/class-frontend.php#L442
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[notice] 450-450: includes/classes/legacy/class-frontend.php#L450
Stand-alone post-increment statement found. Use pre-increment instead: ++$counter. (Universal.Operators.DisallowStandalonePostIncrementDecrement-[fixable])
[warning] 428-428: includes/classes/legacy/class-frontend.php#L428
Inline comments must end in full-stops, exclamation marks, or question marks. (Squiz.Commenting.InlineComment)
[warning] 442-442: includes/classes/legacy/class-frontend.php#L442
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 427-427: includes/classes/legacy/class-frontend.php#L427
Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable])
[warning] 441-441: includes/classes/legacy/class-frontend.php#L441
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 432-432: includes/classes/legacy/class-frontend.php#L432
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 441-441: includes/classes/legacy/class-frontend.php#L441
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 440-440: includes/classes/legacy/class-frontend.php#L440
Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
[warning] 432-432: includes/classes/legacy/class-frontend.php#L432
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])
Description of the Change
Summary by CodeRabbit
register_block_templateto prevent potential errors.Summary by CodeRabbit
New Features
Bug Fixes
Documentation