Collapsible sections #461
Collapsible sections #461
Conversation
|
WalkthroughThis update introduces enhancements to the 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 (4)
- assets/js/blocks/general.js (2)
- assets/js/src/custom.js (2)
- includes/classes/blocks/class-bindings.php (7)
- tour-operator.php (1)
Files not summarized due to errors (1)
- assets/js/blocks/general.js (diff tokens exceeds limit)
Files skipped from review due to trivial changes (2)
- tour-operator.php
- assets/js/blocks/general.js (diff too large)
Review comments generated (9)
- Review: 6
- LGTM: 3
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 = function( button, contentWrapper, limit = 1 ) { | ||
| if ( 0 < contentWrapper.length ) { | ||
| if ( 1 < contentWrapper.children().length ) { |
There was a problem hiding this comment.
The change from a fixed limit to a parameterized limit in the lsx_to.readMoreSet function is a good improvement for flexibility. However, it's important to ensure that all calls to this function have been updated with the new parameter across the codebase. If not, it could lead to unexpected behavior.
- lsx_to.readMoreSet = function( button, contentWrapper ) {
+ lsx_to.readMoreSet = function( button, contentWrapper, limit = 1 ) {| lsx_to.readMoreSet( $(this).find('a'), $(this).closest( '.additional-info' ).find('.content'), 2 ); | ||
| } ); | ||
|
|
||
| $( '.single-tour-operator .additional-info .lsx-to-more-link' ).on( 'click', function( event ) { | ||
| event.preventDefault(); | ||
| $( this ).hide(); | ||
|
|
||
| if ( $( this ).find('a').hasClass( 'less-link' ) ) { | ||
| lsx_to.readMoreSet( $(this).find('a'), $(this).closest( '.additional-info' ).find('.content') ); | ||
| lsx_to.readMoreSet( $(this).find('a'), $(this).closest( '.additional-info' ).find('.content'), 2 ); |
There was a problem hiding this comment.
The changes here reflect the update in the function signature of lsx_to.readMoreSet. The hardcoded limit of '2' is now passed as an argument. This seems fine as long as the limit '2' is the desired behavior in all these cases.
- lsx_to.readMoreSet( $(this).find('a'), $(this).closest( '.additional-info' ).find('.content') );
+ lsx_to.readMoreSet( $(this).find('a'), $(this).closest( '.additional-info' ).find('.content'), 2 );| /** | ||
| * Renders the units block with custom content. | ||
| * | ||
| * This function processes the block content by checking if it belongs to a specific | ||
| * custom block variation and then iteratively builds the units content based on | ||
| * predefined fields and templates. It returns the final rendered block content. | ||
| * | ||
| * @param string $block_content The original content of the block. | ||
| * @param array $parsed_block Parsed data for the block, including type and attributes. | ||
| * @param object $block_obj Block object instance for the current block being processed. | ||
| * | ||
| * @return string Returns the modified block content after processing units data. | ||
| */ |
There was a problem hiding this comment.
The function render_units_block checks if $parsed_block['blockName'] and $parsed_block['attrs'] are set but does not handle the case where they might be empty. This could lead to unexpected behavior. Consider adding a check for empty values as well.
- if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) {
+ if ( ! isset( $parsed_block['blockName'] ) || empty($parsed_block['blockName']) || ! isset( $parsed_block['attrs'] ) || empty($parsed_block['attrs']) ) {| /** | ||
| * Renders the gallery block with custom content. | ||
| * | ||
| * This function processes the block content by checking if it belongs to a specific | ||
| * custom block variation and then iteratively builds the gallery content based on | ||
| * predefined fields and templates. It returns the final rendered block content. | ||
| * | ||
| * @param string $block_content The original content of the block. | ||
| * @param array $parsed_block Parsed data for the block, including type and attributes. | ||
| * @param object $block_obj Block object instance for the current block being processed. | ||
| * | ||
| * @return string Returns the modified block content after processing gallery data. | ||
| */ |
There was a problem hiding this comment.
Similar to the previous comment, consider adding a check for empty values in the render_gallery_block function.
- if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) {
+ if ( ! isset( $parsed_block['blockName'] ) || empty($parsed_block['blockName']) || ! isset( $parsed_block['attrs'] ) || empty($parsed_block['attrs']) ) {| /** | ||
| * Renders the map block with custom content. | ||
| * | ||
| * This function processes the block content by checking if it belongs to a specific | ||
| * custom block variation and then iteratively builds the map content based on | ||
| * predefined fields and templates. It returns the final rendered block content. | ||
| * | ||
| * @param string $block_content The original content of the block. | ||
| * @param array $parsed_block Parsed data for the block, including type and attributes. | ||
| * @param object $block_obj Block object instance for the current block being processed. | ||
| * | ||
| * @return string Returns the modified block content after processing map data. | ||
| */ |
There was a problem hiding this comment.
Again, consider adding a check for empty values in the render_map_block function.
- if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) {
+ if ( ! isset( $parsed_block['blockName'] ) || empty($parsed_block['blockName']) || ! isset( $parsed_block['attrs'] ) || empty($parsed_block['attrs']) ) {|
|
||
| /** | ||
| * Renders the p block with custom content. | ||
| * | ||
| * @param string $block_content The original content of the block. | ||
| * @param array $parsed_block Parsed data for the block, including type and attributes. | ||
| * @param object $block_obj Block object instance for the current block being processed. | ||
| * | ||
| * @return string Returns block with the permalink added. | ||
| */ | ||
| public function render_permalink_block( $block_content, $parsed_block, $block_obj ) { | ||
| // Determine if this is the custom block variation. | ||
| if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) { | ||
| return $block_content; | ||
| } | ||
| $allowed_blocks = array( | ||
| 'core/button' | ||
| ); | ||
|
|
||
| if ( ! in_array( $parsed_block['blockName'], $allowed_blocks, true ) ) { | ||
| return $block_content; | ||
| } | ||
|
|
||
| if ( ! isset( $parsed_block['attrs']['metadata']['name'] ) ) { | ||
| return $block_content; | ||
| } | ||
|
|
||
| if ( 'Permalink' !== $parsed_block['attrs']['metadata']['name'] ) { | ||
| return $block_content; | ||
| } | ||
|
|
||
| $url = get_permalink(); | ||
| $pattern = '/#permalink/s'; | ||
| $block_content = preg_replace( $pattern, $url, $block_content ); | ||
|
|
||
| return $block_content; | ||
| } |
There was a problem hiding this comment.
In the render_permalink_block function, you're using preg_replace to replace #permalink with the actual URL. This could potentially replace unintended parts of the content if #permalink appears elsewhere. Consider using a more specific pattern or another method to replace the placeholder.
Also, you're checking if $parsed_block['attrs']['metadata']['name'] is set but not if it's empty. Consider adding a check for empty values as well.
- if ( ! isset( $parsed_block['attrs']['metadata']['name'] ) ) {
+ if ( ! isset( $parsed_block['attrs']['metadata']['name'] ) || empty($parsed_block['attrs']['metadata']['name']) ) {- $pattern = '/#permalink/s';
+ $pattern = '/^#permalink$/s'; // Adjust the pattern to match your needs
this is a draft, do not review yet.
Summary by CodeRabbit
Release Notes:
readMoreSetfunction incustom.jsto accept a limit parameter, allowing for more flexible control over content visibility.readMoreSetacross the codebase to utilize the new limit functionality, improving content display management.class-bindings.php, expanding the capabilities of block management.tour-operator.phpfor cleaner code structure.