Beta 2 - Bugfixes Round 3 #450
Conversation
krugazul
added
[Type] Bug
|
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 You can disable this status message by setting the 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 introduces enhancements to the WordPress editor, including new block configurations and improved handling of image sizes. It also refines CSS styling and HTML structure for better layout and presentation. Additionally, it optimizes function signatures and logic related to content display, and includes debugging improvements. 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. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Files selected (13)
- assets/css/scss/_icons.scss (1)
- assets/css/scss/_slider.scss (1)
- assets/css/style.css (2)
- assets/js/blocks/general.js (3)
- assets/js/src/custom.js (4)
- includes/classes/admin/class-setup.php (3)
- includes/classes/blocks/class-bindings.php (6)
- includes/classes/blocks/class-registration.php (5)
- includes/classes/legacy/class-accommodation.php (1)
- includes/classes/legacy/class-unit-query.php (1)
- includes/patterns/gallery.php (1)
- includes/patterns/room-card.php (3)
- includes/template-tags/helpers.php (1)
Files ignored due to filter (1)
- vendor/cmb-field-select2/cmb-field-select2.php
Files not summarized due to errors (1)
- assets/css/style.css (diff tokens exceeds limit)
Files not reviewed due to errors (4)
- includes/classes/legacy/class-unit-query.php (no response)
- includes/classes/blocks/class-bindings.php (no response)
- includes/classes/legacy/class-accommodation.php (no response)
- assets/css/style.css (no response)
Files skipped from review due to trivial changes (5)
- assets/css/scss/_icons.scss
- assets/css/scss/_slider.scss
- assets/js/blocks/general.js
- includes/classes/blocks/class-registration.php
- includes/patterns/gallery.php
Review comments generated (12)
- Review: 11
- LGTM: 1
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.
| lsx_to.readMoreSet( $(this), $(this).closest( '.wp-block-group' ).find('.wp-block-post-content') ); | ||
| } ); | ||
|
|
||
| $( '.single-tour-operator .wp-block-read-more' ).on( 'click', function( event ) { | ||
| event.preventDefault(); | ||
| $( this ).hide(); | ||
|
|
||
| if ( $( this ).hasClass( 'less-link' ) ) { | ||
| lsx_to.readMoreSet($( this )); | ||
| lsx_to.readMoreSet( $(this), $(this).closest( '.wp-block-group' ).find('.wp-block-post-content') ); | ||
| } else { | ||
| lsx_to.readMoreOpen($( this )); | ||
| lsx_to.readMoreOpen( $(this), $(this).closest( '.wp-block-group' ).find('.wp-block-post-content') ); | ||
| } | ||
|
|
||
| $( this ).show(); | ||
| } ); | ||
| }; | ||
|
|
||
| lsx_to.readMoreSet = function( button ) { | ||
| let contentWrapper = button.closest( '.wp-block-group' ).find('.wp-block-post-content'); | ||
| lsx_to.readMoreSet = function( button, contentWrapper ) { | ||
| console.log(contentWrapper); | ||
| console.log(contentWrapper.length); | ||
| if ( 0 < contentWrapper.length ) { | ||
| if ( 1 < contentWrapper.find('p').length ) { | ||
| if ( 1 < contentWrapper.children().length ) { | ||
|
|
||
| var limit = 1; | ||
| let counter = 0; | ||
|
|
||
| contentWrapper.find('p').each( function() { | ||
| contentWrapper.children().each( function() { |
There was a problem hiding this comment.
The use of console.log statements on lines 72 and 73 should be removed as they are likely used for debugging purposes and should not be present in production code. Also, the logic to hide elements based on a counter and limit could be optimized by using jQuery's :gt() selector which selects elements with an index greater than a number.
- console.log(contentWrapper);
- console.log(contentWrapper.length);
- if ( 0 < contentWrapper.length ) {
- if ( 1 < contentWrapper.children().length ) {
- var limit = 1;
- let counter = 0;
- contentWrapper.children().each( function() {
- if ( limit <= counter ) {
- $(this).hide();
- }
- counter++;
- });
- }
- }
+ if (contentWrapper && contentWrapper.children().length > 1) {
+ contentWrapper.children(':gt(0)').hide();
+ }| } else { | ||
| button.hide(); | ||
| } | ||
| } | ||
|
|
||
| lsx_to.readMoreOpen = function( button ) { | ||
| let contentWrapper = button.closest( '.wp-block-group' ).find('.wp-block-post-content p'); | ||
| if ( 0 < contentWrapper.length ) { | ||
| contentWrapper.each( function() { | ||
| $(this).show(); | ||
| lsx_to.readMoreOpen = function( button, contentWrapper ) { | ||
| if ( 0 < contentWrapper.children().length ) { | ||
| contentWrapper.children().each( function() { | ||
| if ( ! $(this).hasClass('wp-block-read-more') ) { | ||
| $(this).show(); | ||
| } |
There was a problem hiding this comment.
This block of code is fine, but it would be better to check if the button exists before trying to hide it or change its text. This will prevent potential errors if the button does not exist.
- } else {
- button.hide();
- }
+ } else if (button) {
+ button.hide();
+ }| lsx_to.readMoreSet( $(this), $(this).closest( '.additional-info' ).find('.content') ); | ||
| } ); | ||
|
|
||
| $( '.single-tour-operator .additional-info .lsx-to-more-link' ).on( 'click', function( event ) { | ||
| event.preventDefault(); | ||
| $( this ).hide(); | ||
|
|
||
| if ( $( this ).hasClass( 'less-link' ) ) { | ||
| lsx_to.readMoreSetTI($( this )); | ||
| lsx_to.readMoreSet( $(this), $(this).closest( '.additional-info' ).find('.content') ); | ||
| } else { | ||
| lsx_to.readMoreOpenTI($( this )); | ||
| lsx_to.readMoreOpenTI( $(this), $(this).closest( '.additional-info' ).find('.content') ); | ||
| } | ||
|
|
||
| $( this ).show(); | ||
| } ); |
There was a problem hiding this comment.
Similar to the previous comment, it would be better to check if the element exists before trying to hide it or perform operations on it.
- $( this ).hide();
+ if ($(this)) {
+ $(this).hide();
+ }| $( '.single-tour-operator .lsx-itinerary-wrapper .wp-block-read-more' ).each( function() { | ||
| $(this).show(); | ||
| lsx_to.readMoreItinText = $(this).find('a').text(); | ||
| lsx_to.readMoreSet( $(this), $(this).parent( 'div' ).find('.itinerary-description') ); | ||
| } ); | ||
|
|
||
| $( '.single-tour-operator .lsx-itinerary-wrapper .wp-block-read-more' ).on( 'click', function( event ) { | ||
| event.preventDefault(); | ||
| $( this ).hide(); | ||
|
|
||
| $( this ).parents( '#itinerary' ).find( '.itinerary-item.hidden' ).each( function() { | ||
| $( this ).removeClass( 'hidden' ); | ||
| } ); | ||
| if ( $( this ).hasClass( 'less-link' ) ) { | ||
| lsx_to.readMoreSet( $(this), $(this).parent( 'div' ).find('.itinerary-description') ); | ||
| } else { | ||
| lsx_to.readMoreOpen( $(this), $(this).parent( 'div' ).find('.itinerary-description') ); | ||
| } | ||
|
|
||
| $( this ).show(); |
There was a problem hiding this comment.
Again, it would be better to check if the element exists before trying to hide it or perform operations on it.
- $( this ).hide();
+ if ($(this)) {
+ $(this).hide();
+ }| @@ -378,7 +353,7 @@ | |||
| $document.ready( function() { | |||
| lsx_to.set_read_more(); | |||
| lsx_to.set_read_more_travel_info(); | |||
| //lsx_to.set_read_more_itinerary(); | |||
| lsx_to.set_read_more_itinerary(); | |||
There was a problem hiding this comment.
This line is fine as it is. It's just calling a function when the document is ready, which is a common practice in jQuery.
- lsx_to.set_read_more_itinerary();
+ lsx_to.set_read_more_itinerary();|
|
||
| /** | ||
| * Register the image sizes with WordPress | ||
| * | ||
| * @return void | ||
| */ | ||
| public function register_image_sizes() { | ||
| foreach ( $this->image_sizes as $key => $params ) { | ||
| add_image_size( $key, $params['width'], $params['height'], $params['crop'] ); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * The array of image sizes from WordPress | ||
| * | ||
| * @param array $sizes | ||
| * @return array | ||
| */ | ||
| public function editor_image_sizes( $sizes ) { | ||
| $new_sizes = []; | ||
| foreach ( $this->image_sizes as $key => $params ) { | ||
| $new_sizes[ $key ] = $params['title']; | ||
| } | ||
| return array_merge( $sizes, $new_sizes ); | ||
| } |
There was a problem hiding this comment.
This code registers the image sizes with WordPress and adds them to the editor options. It's a good practice to check if the $params['title'] exists before using it to avoid PHP notices.
public function editor_image_sizes( $sizes ) {
$new_sizes = [];
foreach ( $this->image_sizes as $key => $params ) {
if (isset($params['title'])) {
$new_sizes[ $key ] = $params['title'];
}
}
return array_merge( $sizes, $new_sizes );
}| $value = array_filter($value); | ||
| $value = implode( ', ', $value ); | ||
| } |
There was a problem hiding this comment.
The addition of array_filter before imploding the array is a good practice as it removes any empty or false values from the array. This can prevent issues where you might end up with extra commas due to empty values in your final string.
+ $value = array_filter($value);
$value = implode( ', ', $value );This change improves the robustness of the code by ensuring that only meaningful data (non-empty values) are processed.
| <!-- wp:group {"className":"unit-type-wrapper","style":{"spacing":{"margin":{"top":"0","bottom":"0"},"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} --> | ||
| <div class="wp-block-group unit-type-wrapper" style="margin-top:0;margin-bottom:0"><!-- wp:group {"style":{"spacing":{"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap","verticalAlignment":"top"}} --> |
There was a problem hiding this comment.
The class name has been changed from itin-type-wrapper to unit-type-wrapper. This change should be fine as long as it is consistent across all the files and the new class unit-type-wrapper is defined in the CSS with the required properties. If not, it might break the styling.
- <!-- wp:group {"className":"itin-type-wrapper","style":{"spacing":{"margin":{"top":"0","bottom":"0"},"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} -->
- <div class="wp-block-group itin-type-wrapper" style="margin-top:0;margin-bottom:0">
+ <!-- wp:group {"className":"unit-type-wrapper","style":{"spacing":{"margin":{"top":"0","bottom":"0"},"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} -->
+ <div class="wp-block-group unit-type-wrapper" style="margin-top:0;margin-bottom:0">| <!-- wp:group {"className":"unit-price-wrapper","style":{"spacing":{"margin":{"top":"0","bottom":"0"},"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} --> | ||
| <div class="wp-block-group unit-price-wrapper" style="margin-top:0;margin-bottom:0"><!-- wp:group {"style":{"spacing":{"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap","verticalAlignment":"top"}} --> |
There was a problem hiding this comment.
Similar to the previous comment, the class name has been changed from itin-type-wrapper to unit-price-wrapper. Ensure that this change is consistent across all the files and the new class unit-price-wrapper is defined in the CSS with the required properties.
- <!-- wp:group {"className":"itin-type-wrapper","style":{"spacing":{"margin":{"top":"0","bottom":"0"},"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} -->
- <div class="wp-block-group itin-type-wrapper" style="margin-top:0;margin-bottom:0">
+ <!-- wp:group {"className":"unit-price-wrapper","style":{"spacing":{"margin":{"top":"0","bottom":"0"},"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} -->
+ <div class="wp-block-group unit-price-wrapper" style="margin-top:0;margin-bottom:0">| @@ -63,7 +63,7 @@ | |||
| <!-- /wp:group --> | |||
|
|
|||
| <!-- wp:group {"style":{"spacing":{"blockGap":"5px"}},"layout":{"type":"flex","flexWrap":"nowrap"}} --> | |||
| <div class="wp-block-group"><!-- wp:paragraph {"className":"unit-price","style":{"elements":{"link":{"color":{"text":"var:preset|color|primary-700"}}},"spacing":{"padding":{"top":"2px","bottom":"2px"}}},"textColor":"primary-700"} --> | |||
| <div class="wp-block-group"><!-- wp:paragraph {"className":"unit-price amount","style":{"elements":{"link":{"color":{"text":"var:preset|color|primary-700"}}},"spacing":{"padding":{"top":"2px","bottom":"2px"}}},"textColor":"primary-700"} --> | |||
There was a problem hiding this comment.
The class name amount has been added to the paragraph element. This change should be fine as long as it is consistent across all the files and the new class amount is defined in the CSS with the required properties.
- <p class="unit-price has-primary-700-color has-text-color has-link-color" style="padding-top:2px;padding-bottom:2px"></p>
+ <p class="unit-price amount has-primary-700-color has-text-color has-link-color" style="padding-top:2px;padding-bottom:2px"></p>
Description of the Change
The following PR contains a third round of bug fixes as well as a some items to close off some issues.
Alternate Designs
Itinerary Read More
Rooms showing a currency symbol
Images sizes showing via the Regenerate Thumbnails plugin
Related Tours with fixed currency
Related Accommodation card with fixed currency.
Verification Process (once deployed)
Summary by CodeRabbit
item_thumbnailmethod to accept size parameter, providing flexibility in image retrieval.