2.0 Beta 1 #442
2.0 Beta 1 #442
Conversation
…n of the custom fields.
Settings Restructure and Refactoring
…ibility-fixes Read more block, visibility fixes and slider trigger
…ations Registering query loop variations
…rguments 431 query loop variations arguments
Fixing the Travel info read more.
WalkthroughThe changes in this pull request involve extensive modifications across various files, primarily focusing on the removal of obsolete SCSS files, the introduction of new JavaScript functionalities for WordPress block variations, and updates to existing PHP classes. Key files include the addition of new settings fields, adjustments to the admin interface, and the restructuring of the Tour Operator plugin's architecture. Additionally, multiple legacy classes have been removed, indicating a shift towards a more streamlined and maintainable codebase. Changes
Poem
Warning Tool Failures:Tool Failure Count: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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (133)
assets/css/style.scss (2)
1-4: 🧹 Nitpick (assertive)
Brilliant header, but could use more details, hey!
The header identifies the component nicely, but consider adding more documentation about:
- The purpose of these styles
- Any dependencies
- Version information
9-14: 🧹 Nitpick (assertive)
Lekker styling structure, but could be more reusable!
The nested
.hiddenclass is properly implemented, but since it's used in multiple wrappers, consider making it a standalone utility class or a mixin for better reusability.Here's a more reusable approach:
+// Utility class for hiding elements +.hidden { + display: none; +} + .lsx-itinerary-wrapper, -.lsx-units-wrapper { - .hidden { - display: none; - } -} +.lsx-units-wrapper { + // Add wrapper-specific styles here if needed +}📝 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.// Utility class for hiding elements .hidden { display: none; } .lsx-itinerary-wrapper, .lsx-units-wrapper { // Add wrapper-specific styles here if needed }.gitignore (1)
7-14: 🛠️ Refactor suggestion
Consolidate duplicate
.DS_Storeentries and improve pattern matching, chommie!Strewth! You've got quite a few duplicate entries there. Let's clean this up properly:
- Multiple
.DS_Storeentries are redundant as a single entry will match all instances- The vendor-specific
.DS_Storeentries are unnecessary when using a global patternHere's a more efficient way to handle these patterns:
-vendor/cmb-field-select2/.DS_Store -vendor/cmb2-field-map/.DS_Store -vendor/.DS_Store -.DS_Store -vendor/.DS_Store -.DS_Store -.DS_Store -vendor/.DS_Store +# OS generated files +.DS_Store +.DS_Store? +._* +.Spotlight-V100 +.Trashes +ehthumbs.db +Thumbs.dbThis change will:
- Remove all duplicates
- Catch
.DS_Storefiles anywhere in the project- Include other common OS-generated files for good measure
📝 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.# OS generated files .DS_Store .DS_Store? ._* .Spotlight-V100 .Trashes ehthumbs.db Thumbs.db.mergify.yml (1)
19-19: 🧹 Nitpick (assertive)
Eish! Add a newline at the end of the file, hey.
The YAML file is missing a newline character at the end, which is a common requirement for text files.
merge: - method: merge + method: merge +📝 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.method: merge🧰 Tools
🪛 yamllint
[error] 19-19: no new line character at the end of file
(new-line-at-end-of-file)
includes/classes/blocks/class-patterns.php (5)
21-28: 🧹 Nitpick (assertive)
Fix comment formatting and add method documentation.
The constructor requires proper comment formatting and comprehensive documentation.
public function __construct() { - //Register our categories + // Register our categories add_filter( 'block_categories_all', array( $this, 'register_block_category' ), 10, 1 ); add_action( 'init', array( $this, 'register_block_pattern_category' ) ); - // Register our block patterns + // Register block patterns add_action( 'init', array( $this, 'register_block_patterns' ), 10 ); }Committable suggestion skipped: line range outside the PR's diff.
35-41: 🧹 Nitpick (assertive)
Enhance method documentation and add type hints.
The method would benefit from improved documentation and type safety.
/** - * Registers the projects pattern category + * Registers the LSX Tour Operator block category. + * + * @param array $categories Existing block categories. + * @return array Modified block categories. + */ - public function register_block_category( $categories ) { + public function register_block_category( array $categories ): array {📝 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./** * Registers the LSX Tour Operator block category. * * @param array $categories Existing block categories. * @return array Modified block categories. */ public function register_block_category( array $categories ): array { $categories[] = array( 'slug' => $this->category, 'title' => __( 'LSX Tour Operator', 'lsx-tour-operator' ) ); return $categories; }
1-12: 🧹 Nitpick (assertive)
Add comprehensive file-level documentation.
Kindly add a file-level docblock to describe the purpose and functionality of this class. This will improve code maintainability and align with WordPress coding standards.
<?php +/** + * LSX Tour Operator Block Patterns Class. + * + * @package LSX Tour Operator + * @author LightSpeed + * @license GPL3 + * @link https://lsx.lsdev.biz/ + * @copyright 2024 LightSpeed + */ namespace lsx\blocks;Additionally, consider enhancing the property documentation:
/** * Holds the slug of the projects pattern category. + * + * This slug is used to register and identify block patterns + * specific to the LSX Tour Operator plugin. * * @var string */📝 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.<?php /** * LSX Tour Operator Block Patterns Class. * * @package LSX Tour Operator * @author LightSpeed * @license GPL3 * @link https://lsx.lsdev.biz/ * @copyright 2024 LightSpeed */ namespace lsx\blocks; class Patterns { /** * Holds the slug of the projects pattern category. * * This slug is used to register and identify block patterns * specific to the LSX Tour Operator plugin. * * @var string */ private $category = 'lsx-tour-operator';🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 1-1: includes/classes/blocks/class-patterns.php#L1
Missing file doc comment. (Squiz.Commenting.FileComment)
48-53: 🛠️ Refactor suggestion
Add error handling for pattern category registration.
Consider adding error handling to manage potential registration failures.
public function register_block_pattern_category() { - register_block_pattern_category( - $this->category, - array( 'label' => __( 'LSX Tour Operator', 'lsx-tour-operator' ) ) - ); + $result = register_block_pattern_category( + $this->category, + array( 'label' => __( 'LSX Tour Operator', 'lsx-tour-operator' ) ) + ); + + if ( \is_wp_error( $result ) ) { + error_log( sprintf( 'Failed to register LSX Tour Operator pattern category: %s', $result->get_error_message() ) ); + } }📝 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.public function register_block_pattern_category() { $result = register_block_pattern_category( $this->category, array( 'label' => __( 'LSX Tour Operator', 'lsx-tour-operator' ) ) ); if ( \is_wp_error( $result ) ) { error_log( sprintf( 'Failed to register LSX Tour Operator pattern category: %s', $result->get_error_message() ) ); } }
60-73:
⚠️ Potential issueImplement security measures and error handling.
The current implementation requires additional security measures and error handling.
public function register_block_patterns() { $directory = LSX_TO_PATH . '/includes/patterns/'; + + // Validate directory exists and is readable + if ( ! is_dir( $directory ) || ! is_readable( $directory ) ) { + error_log( sprintf( 'Pattern directory not accessible: %s', $directory ) ); + return; + } - foreach ( glob( $directory . '*.php') as $file ) { + $pattern_files = glob( $directory . '*.php' ); + if ( false === $pattern_files ) { + error_log( 'Failed to scan pattern directory' ); + return; + } + + foreach ( $pattern_files as $file ) { + // Validate file is readable and has .php extension + if ( ! is_readable( $file ) || '.php' !== substr( $file, -4 ) ) { + continue; + } + // Extract the filename without the directory path and extension $filename = basename( $file, '.php' ); // Use the filename to create the key $key = 'lsx-tour-operator/' . $filename; - - // Require the file and add it to the patterns array - register_block_pattern( $key, require $file ); + + try { + $pattern = require $file; + if ( ! is_array( $pattern ) ) { + error_log( sprintf( 'Invalid pattern data in file: %s', $file ) ); + continue; + } + register_block_pattern( $key, $pattern ); + } catch ( \Exception $e ) { + error_log( sprintf( 'Failed to register pattern %s: %s', $key, $e->getMessage() ) ); + } } }📝 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.public function register_block_patterns() { $directory = LSX_TO_PATH . '/includes/patterns/'; // Validate directory exists and is readable if ( ! is_dir( $directory ) || ! is_readable( $directory ) ) { error_log( sprintf( 'Pattern directory not accessible: %s', $directory ) ); return; } $pattern_files = glob( $directory . '*.php' ); if ( false === $pattern_files ) { error_log( 'Failed to scan pattern directory' ); return; } foreach ( $pattern_files as $file ) { // Validate file is readable and has .php extension if ( ! is_readable( $file ) || '.php' !== substr( $file, -4 ) ) { continue; } // Extract the filename without the directory path and extension $filename = basename( $file, '.php' ); // Use the filename to create the key $key = 'lsx-tour-operator/' . $filename; try { $pattern = require $file; if ( ! is_array( $pattern ) ) { error_log( sprintf( 'Invalid pattern data in file: %s', $file ) ); continue; } register_block_pattern( $key, $pattern ); } catch ( \Exception $e ) { error_log( sprintf( 'Failed to register pattern %s: %s', $key, $e->getMessage() ) ); } } }🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 67-67: includes/classes/blocks/class-patterns.php#L67
Inline comments must end in full-stops, exclamation marks, or question marks. (Squiz.Commenting.InlineComment)assets/js/blocks/slider-query.js (3)
1-53: 🧹 Nitpick (assertive)
Consider restructuring for better maintainability, china.
To improve the overall architecture:
- Split the code into separate modules (HOC, filters, constants)
- Add JSDoc documentation for better code clarity
- Consider adding unit tests for the component and filters
42-51: 🧹 Nitpick (assertive)
Add proper class name handling, boet.
The class name management in the save content filter needs improvement:
- Add validation for existing class names to prevent duplicates
- Handle class removal when slider is disabled
- Consider using classnames utility for better class management
Here's a better implementation:
+ import classNames from 'classnames'; + wp.hooks.addFilter( 'blocks.getSaveContent.extraProps', 'lsx-tour-operator/save-slider-panel', function (extraProps, blockType, attributes) { - if (blockType.name === 'core/query' && attributes.hasCustomClass) { - extraProps.className = (extraProps.className || '') + ' lsx-to-slider'; + if (blockType.name === 'core/query') { + extraProps.className = classNames(extraProps.className, { + 'lsx-to-slider': attributes.isSliderEnabled + }); } return extraProps; } );Committable suggestion skipped: line range outside the PR's diff.
7-34: 🛠️ Refactor suggestion
Revise attribute naming and add proper validation, my friend.
The higher-order component implementation could be improved in the following ways:
- The attribute name
hasCustomClassisn't descriptive of its purpose. ConsiderisSliderEnabledinstead.- Add prop-types validation for component props.
- Consider handling cleanup when the slider is disabled.
Here's how to improve it:
+ import PropTypes from 'prop-types'; + var withInspectorControls = wp.compose.createHigherOrderComponent(function (BlockEdit) { return function (props) { if (props.name !== 'core/query') { return el(BlockEdit, props); } - var hasCustomClass = props.attributes.hasCustomClass || false; + var isSliderEnabled = props.attributes.isSliderEnabled || false; return el( element.Fragment, {}, el(BlockEdit, props), el(InspectorControls, {}, el(PanelBody, { title: 'Slider', initialOpen: true }, el(CheckboxControl, { label: 'Enable Slider', - checked: hasCustomClass, + checked: isSliderEnabled, onChange: function (value) { props.setAttributes({ - hasCustomClass: value + isSliderEnabled: value }); } }) ) ) ); }; }, 'withInspectorControls'); + withInspectorControls.propTypes = { + name: PropTypes.string.isRequired, + attributes: PropTypes.shape({ + isSliderEnabled: PropTypes.bool + }), + setAttributes: PropTypes.func.isRequired + };Committable suggestion skipped: line range outside the PR's diff.
assets/js/blocks/slotfills.js (4)
56-84: 🧹 Nitpick (assertive)
Consider using JSX for better readability, hey
The nested
createElementcalls make the code harder to follow. Using JSX would make the component structure more intuitive and maintainable.const LsxToStatusPlugin = () => ( <PluginPostStatusInfo name="lsx-to-toggles" className="lsx-to-toggles" > <div className="lsx-to-toggles__inner"> <div className="toggle-row"> <StickyToggle /> </div> <div className="toggle-row"> <DisableSingleToggle /> </div> </div> </PluginPostStatusInfo> );
86-89: 🧹 Nitpick (assertive)
Proper plugin registration, sharp!
The plugin registration is implemented correctly. Consider adding an icon to improve visual identification in the WordPress editor interface.
registerPlugin('lsx-to-status-plugin', { render: LsxToStatusPlugin, - icon: null, + icon: createElement(wp.components.Icon, { icon: 'admin-settings' }), });📝 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.registerPlugin('lsx-to-status-plugin', { render: LsxToStatusPlugin, icon: createElement(wp.components.Icon, { icon: 'admin-settings' }), });
33-54: 🛠️ Refactor suggestion
Refactor to reduce code duplication, my bru
The
DisableSingleTogglecomponent shares nearly identical logic withStickyToggle. Consider creating a reusable toggle component to improve maintainability.+const MetaToggle = ({ metaKey, label }) => { + const { editPost } = useDispatch("core/editor"); + const handleChange = (newChecked) => { + editPost({ meta: { [metaKey]: newChecked } }); + }; + + const isEnabled = useSelect( function ( select ) { + return select( 'core/editor' ).getEditedPostAttribute( + 'meta' + )[ metaKey ]; + }, [] ); + + return createElement( + ToggleControl, + { + label: i18n.__(label), + checked: isEnabled, + onChange: handleChange + } + ); +}; + -const StickyToggle = () => { - // ... current implementation -}; +const StickyToggle = () => createElement(MetaToggle, { + metaKey: 'featured', + label: 'Featured' +}); -const DisableSingleToggle = () => { - // ... current implementation -}; +const DisableSingleToggle = () => createElement(MetaToggle, { + metaKey: 'disable_single', + label: 'Disable Single' +});Committable suggestion skipped: line range outside the PR's diff.
11-31: 🧹 Nitpick (assertive)
Improve variable naming for better clarity, boet
The
isStickyvariable name doesn't clearly represent the "featured" status it's tracking. Consider renaming it toisFeaturedto better reflect its purpose.- const isSticky = useSelect( function ( select ) { + const isFeatured = useSelect( function ( select ) { return select( 'core/editor' ).getEditedPostAttribute( 'meta' )[ 'featured' ]; }, [] ); return createElement( ToggleControl, { label: i18n.__('Featured'), - checked: isSticky, + checked: isFeatured, onChange: handleChange } );📝 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.const StickyToggle = () => { const { editPost } = useDispatch("core/editor"); const handleChange = (newChecked) => { editPost({ meta: { 'featured': newChecked } }); }; const isFeatured = useSelect( function ( select ) { return select( 'core/editor' ).getEditedPostAttribute( 'meta' )[ 'featured' ]; }, [] ); return createElement( ToggleControl, { label: i18n.__('Featured'), checked: isFeatured, onChange: handleChange } ); };assets/js/src/admin.js (1)
70-80: 🧹 Nitpick (assertive)
Eish, we've got mixed class naming conventions here!
The code uses both
ui-tabandnav-tabclasses, which is a bit confusing. Let's stick to WordPress admin conventions throughout:-jQuery('.ui-tab.active').removeClass('active'); +jQuery('.nav-tab-content.active').removeClass('active');Also, consider adding a fallback for when the target tab panel doesn't exist:
var targetPanel = jQuery($this.attr('href')); if (targetPanel.length) { targetPanel.addClass('active'); }assets/css/scss/_icons.scss (5)
1-104: 🧹 Nitpick (assertive)
Consider refactoring currency symbol definitions
The implementation is functional but could benefit from these improvements:
- Group similar currency symbols together (e.g., all Latin letter-based symbols)
- Consider using SCSS variables for margin values for easier maintenance
Example refactor:
$currency-margin: 3px; %currency-icons { .currency-icon { &:after { margin-left: $currency-margin; margin-right: $currency-margin; } // Symbol-based currencies &.usd, &.nad, &.cad, &.hkd, &.sgd, &.nzd, &.aud { &:after { content: '\0024'; } } // Letter-based currencies &.zar, &.bwp, &.kes { // ... group these together } // Multi-letter currencies &.chf, &.tzs, &.mwk { // ... group these together } } }
133-134: 🛠️ Refactor suggestion
Remove commented destination wrapper code
Remove this commented code as it's no longer needed in the 2.0.0 update.
-// Add the icon ULR here. -//.lsx-destination-to-tour-wrapper::before { background-image: url( ../img/icons/destinations-icon.webp ); }📝 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.
106-117: 🛠️ Refactor suggestion
Remove commented legacy code
As part of the 2.0.0 update, this commented block should be removed rather than kept. If needed, the code history can be referenced through version control.
-// Add the block class name here to create spacing for the icon. -/*.lsx-price-wrapper, -.lsx-destination-to-tour-wrapper { - &:before { - background-repeat: no-repeat; - background-size: contain; - width: 20px; - height: 30px; - content: " "; - background-position: center top; - } -}*/📝 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.
47-49:
⚠️ Potential issueFix incorrect currency symbol for Indonesian Rupiah (IDR)
The IDR currency symbol is incorrectly using the Indian Rupee symbol (\20B9). The correct symbol for Indonesian Rupiah is "Rp" which should be "\0052\0070".
- &:after { content: '\20B9'; } + &:after { content: '\0052\0070'; }📝 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.&.idr { &:after { content: '\0052\0070'; } }
119-131: 🧹 Nitpick (assertive)
Clean up price wrapper implementation
The implementation looks good but needs these minor improvements:
- Remove commented legacy code
- Use
0instead of0pxfor margin value.lsx-price-wrapper { - /*&:before { - background-image: url( ../img/icons/price-icon.png ); - }*/ - & .amount { @extend %currency-icons; .currency-icon { - &:after { margin-right: 0px; } + &:after { margin-right: 0; } } } }📝 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..lsx-price-wrapper { & .amount { @extend %currency-icons; .currency-icon { &:after { margin-right: 0; } } } }includes/classes/legacy/class-video.php (2)
69-74:
⚠️ Potential issueResolve conflicting sortable settings, chap!
There's a bit of a contradiction in your code here. You've got
'sortable' => trueat the group level, but then'sortable' => falsein the options. This could lead to confusion about whether videos can be reordered or not.Let's sort this out with a proper fix:
'repeatable' => true, - 'sortable' => true, + 'sortable' => false, 'options' => array( 'group_title' => __( 'Video {#}', 'tour-operator' ), 'add_button' => __( 'Add Video', 'tour-operator' ), 'remove_button' => __( 'Remove Video', 'tour-operator' ), 'sortable' => false,📝 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.'sortable' => false, 'options' => array( 'group_title' => __( 'Video {#}', 'tour-operator' ), // since version 1.1.4, {#} gets replaced by row number 'add_button' => __( 'Add Video', 'tour-operator' ), 'remove_button' => __( 'Remove Video', 'tour-operator' ), 'sortable' => false,
83-98: 🧹 Nitpick (assertive)
Consider enhancing field types for better user experience, mate!
The fields are well-structured, but a few tweaks could make them more user-friendly:
- The URL field should validate video URLs
- The description field might benefit from a larger input area
Here's a suggested improvement:
array( 'id' => 'url', 'name' => __( 'Url', 'tour-operator' ), 'desc' => __( 'The URL link to the video.', 'tour-operator' ), - 'type' => 'text', + 'type' => 'url', ), array( 'id' => 'description', 'name' => __( 'Caption', 'tour-operator' ), 'desc' => __( 'The caption displayed with the video on the frontend.', 'tour-operator' ), - 'type' => 'text', + 'type' => 'textarea_small', ),📝 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.array( 'id' => 'url', 'name' => __( 'Url', 'tour-operator' ), 'desc' => __( 'The URL link to the video.', 'tour-operator' ), 'type' => 'url', ), array( 'id' => 'description', 'name' => __( 'Caption', 'tour-operator' ), 'desc' => __( 'The caption displayed with the video on the frontend.', 'tour-operator' ), 'type' => 'textarea_small', ), array( 'id' => 'alt_text', 'name' => __( 'Alt Text', 'tour-operator' ), 'desc' => __( 'Alternative text for accessibility, describing the video for screen readers.', 'tour-operator' ),includes/classes/admin/class-pages.php (1)
75-75: 🧹 Nitpick (assertive)
Lekker addition of the action hook!
The new action hook enhances the plugin's extensibility by allowing external code to register menu pages. This aligns perfectly with the plugin's rewrite goals.
Consider adding a code comment above the action hook to document its purpose and usage:
+// Allow external code to register additional menu pages before creation do_action( 'lsx_to_register_menu_pages' );📝 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.// Allow external code to register additional menu pages before creation do_action( 'lsx_to_register_menu_pages' );assets/css/scss/admin.scss (2)
60-74: 🧹 Nitpick (assertive)
Lekker implementation of form fields and tabs!
The max-width constraint on form fields and tab visibility controls are well implemented. However, consider making the selectors more specific to prevent potential style conflicts:
-.wrap.lsx-to-settings { +.wp-admin .wrap.lsx-to-settings { .form-field { input[type="text"], input[type="number"], select { max-width: 300px; } }Committable suggestion skipped: line range outside the PR's diff.
54-58: 🧹 Nitpick (assertive)
Consider improving spinner contrast for better visibility
Howzit! The white background on the spinner might be difficult to see against light backgrounds. Consider using a semi-transparent or contrasting background colour to ensure better visibility.
.wrap.lsx-to-settings { .spinner { - background-color: #fff; + background-color: rgba(0, 0, 0, 0.1); border-radius: 10px; }📝 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..wrap.lsx-to-settings { .spinner { background-color: rgba(0, 0, 0, 0.1); border-radius: 10px; }includes/classes/class-tour-operator.php (2)
7-12: 🧹 Nitpick (assertive)
Well-structured imports supporting new architecture!
The new imports from
lsx\adminandlsx\blocksnamespaces reflect a proper separation of concerns, aligning well with the WordPress site editor integration objective.Consider documenting these architectural changes in the plugin's technical documentation to help other developers understand the new structure.
220-226: 🛠️ Refactor suggestion
Standardise class instantiation pattern
There's an inconsistent pattern in how classes are instantiated:
- Direct instantiation:
Admin,Bindings,Registration,Patterns,Templates- Static initialization:
Settings,SetupThis mixing of patterns could lead to maintenance challenges.
Consider standardising to one approach. If singleton pattern is needed, use static initialization consistently. Otherwise, prefer direct instantiation:
- $this->admin = new Admin(); - $this->settings = Settings::init(); - $this->setup = Setup::init(); - $this->bindings = new Bindings(); - $this->registration = new Registration(); - $this->patterns = new Patterns(); - $this->templates = new Templates(); + $this->admin = new Admin(); + $this->settings = new Settings(); + $this->setup = new Setup(); + $this->bindings = new Bindings(); + $this->registration = new Registration(); + $this->patterns = new Patterns(); + $this->templates = new Templates();Committable suggestion skipped: line range outside the PR's diff.
includes/classes/blocks/class-templates.php (3)
19-28: 🧹 Nitpick (assertive)
Fix documentation access level
The documentation indicates private access (@access private), but the constructor is declared as public. Please update the documentation to match the actual implementation.
- * @access private + * @access public📝 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./** * Initialize the plugin by setting localization, filters, and administration functions. * * @since 1.0.0 * * @access public */ public function __construct() { add_action( 'init', [ $this, 'register_post_type_templates' ] ); }
116-126: 🛠️ Refactor suggestion
Enhance method documentation and security
- The @return void documentation is incorrect as the method returns a string.
- Consider sanitizing the template path to prevent directory traversal.
/** * Gets the PHP template file and returns the content. * - * @param [type] $template - * @return void + * @param string $template The template filename + * @return string The template content */ protected function get_template_content( $template ) { + $template = basename( $template ); ob_start(); include LSX_TO_PATH . "/templates/{$template}"; return ob_get_clean(); }📝 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./** * Gets the PHP template file and returns the content. * * @param string $template The template filename * @return string The template content */ protected function get_template_content( $template ) { $template = basename( $template ); ob_start(); include LSX_TO_PATH . "/templates/{$template}"; return ob_get_clean(); }🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 120-120: includes/classes/blocks/class-templates.php#L120
Function return type is void, but function contains return statement. (Squiz.Commenting.FunctionComment)
103-114: 🧹 Nitpick (assertive)
Consider adding template existence verification
The
get_template_content()method assumes template files exist. Consider adding a file existence check before including the template.foreach ( $post_types as $key => $labels ) { + $template_file = LSX_TO_PATH . "/templates/{$key}.html"; + if ( ! file_exists( $template_file ) ) { + continue; + } $args = [ 'title' => $labels['title'], 'description' => $labels['description'], 'content' => $this->get_template_content( $key . '.html' ), ];Committable suggestion skipped: line range outside the PR's diff.
includes/constants/settings-fields.php (4)
3-21: 🧹 Nitpick (assertive)
Consider adding more African currencies
Howzit! The currency selection is a bit limited for African tour operators. Consider adding these currencies:
- BWP (Botswana Pula)
- KES (Kenyan Shilling)
- MUR (Mauritian Rupee)
- TZS (Tanzanian Shilling)
54-73: 🛠️ Refactor suggestion
Remove duplicated placeholder settings
Shame, we've got the same placeholder settings duplicated in both root level and post_types. Let's refactor this to use a shared configuration.
- 'placeholder' => array( - 'featured_placeholder' => array( - 'label' => esc_html__( 'Upload a placeholder', 'tour-operator' ), - 'type' => 'image', - 'default' => 0, - 'preview_w' => 300, - ), - 'map_placeholder_enabled' => array( - 'label' => esc_html__( 'Enable Map Placeholder', 'tour-operator' ), - 'desc' => esc_html__( 'Enable a placeholder users will click to load the map.', 'tour-operator' ), - 'type' => 'checkbox', - 'default' => 0, - ), - 'map_placeholder' => array( - 'label' => esc_html__( 'Upload a map placeholder', 'tour-operator' ), - 'type' => 'image', - 'default' => 0, - 'preview_w' => 300, - ), - ),Also applies to: 106-125
99-104:
⚠️ Potential issueEnhance Google Maps API key security
Sharp! We need to:
- Add API key format validation
- Store the key securely using WordPress's encryption functions
- Add usage restrictions hint in the description
74-98: 🛠️ Refactor suggestion
Add colour validation for fusion tables
Ja, we need to validate the hex colour inputs. Add a regex pattern or use WordPress's built-in colour picker component.
'fusion_tables_colour_border' => array( 'label' => esc_html__( 'Border Colour', 'tour-operator' ), 'desc' => esc_html__( 'Default value: #000000', 'tour-operator' ), - 'type' => 'text', + 'type' => 'color', 'default' => '#000000', ),Committable suggestion skipped: line range outside the PR's diff.
includes/classes/legacy/class-itinerary-query.php (3)
173-179: 🛠️ Refactor suggestion
Standardise array handling across the codebase.
For consistency with the gallery handling, let's use WordPress's built-in functions here too.
-if ( ! is_array( $room_images ) ) { - $room_images = array( $room_images ); -} +$room_images = wp_parse_id_list( $room_images );📝 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.$room_images = get_post_meta( $accommodation_id, 'units', true ); if ( false !== $room_images && ! empty( $room_images ) ) { $room_images = wp_parse_id_list( $room_images );
145-152: 🧹 Nitpick (assertive)
Enhance array type checking robustness.
The new array type checking is good, but we can make it more robust using
wp_parse_id_list()for consistent array handling of WordPress IDs.-if ( ! is_array( $gallery ) ) { - $gallery = array( $gallery ); -} else { - $gallery = array_keys( $gallery ); -} +$gallery = wp_parse_id_list( $gallery );📝 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.$gallery = get_post_meta( $accommodation_id, 'gallery', true ); if ( false !== $gallery && ! empty( $gallery ) ) { $gallery = wp_parse_id_list( $gallery );
103-103: 💡 Codebase verification
⚠️ Potential issueChanging get_post_meta parameter will cause compatibility issues
The change to
trueinget_post_meta()will break compatibility withclass-lsx-to-tour-schema.php, which explicitly expects an array by usingfalseparameter and iterates through the results with a foreach loop.
includes/classes/legacy/schema/class-lsx-to-tour-schema.phpexpects array return type:$itinerary = get_post_meta( get_the_ID(), 'itinerary', false ); foreach ( $itinerary as $day )🔗 Analysis chain
Verify the impact of get_post_meta return type change.
Changing the third parameter to
truealters the return type from array to single value. Since this is legacy code, we should ensure this doesn't break existing functionality.Consider wrapping this change with a compatibility layer to maintain backwards compatibility.
Let me gather more context about how the itinerary data is used.
Let me try a different approach to check the itinerary usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for code that might expect an array return type from get_post_meta rg -A 3 "get_post_meta.*itinerary.*false"Length of output: 395
Script:
#!/bin/bash # Check how itinerary data is processed in both files ast-grep --pattern 'get_post_meta($_, "itinerary"' # Also check for array operations on itinerary data rg -B 2 -A 4 "\$itinerary\[" includes/classes/legacy/Length of output: 107
Script:
#!/bin/bash # Search for itinerary variable usage with more context rg -B 3 -A 5 "\$this->itineraries" includes/classes/legacy/ # Check for array operations on the itinerary meta rg -B 3 -A 5 "itinerary.*=.*get_post_meta" includes/classes/legacy/Length of output: 962
includes/classes/legacy/class-schema.php (1)
124-125: 💡 Codebase verification
Inconsistent coordinate key usage in legacy map class
There's a mix of old and new coordinate key formats in
includes/classes/legacy/class-maps.php. The file uses 'latitude'/'longitude' in the array keys but 'lat'/'long' in the data attributes:
- Line with
data-latanddata-long:$map .= ' data-lat="' . $args['latitude'] . '" data-long="' . $args['longitude'] . '"';- Multiple instances in map-data divs using
data-latanddata-longFor consistency's sake, we should update these data attributes to use the full forms.
🔗 Analysis chain
Proper standardisation of geographical coordinate keys!
Brilliant work on standardising the coordinate keys to 'latitude' and 'longitude'. This makes the code more professional and easier to understand.
Let's check if we've caught all instances of the old coordinate keys:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining instances of the old coordinate keys echo "Checking for old coordinate key usage..." rg -i "['\"](lat|long)['\"]" --type php echo "Checking for new coordinate key usage..." rg -i "['\"](latitude|longitude)['\"]" --type phpLength of output: 3952
includes/classes/legacy/class-frontend.php (4)
215-245: 🧹 Nitpick (assertive)
Consider making slider parameters configurable via WordPress options
The hardcoded slider parameters could be made more flexible by moving them to the plugin's settings page.
Consider:
- Moving these parameters to the plugin settings
- Adding filters for individual parameter groups
- Implementing responsive breakpoint configurations
197-203: 🛠️ Refactor suggestion
Remove commented out debug conditional block
The commented out debug conditional block serves no purpose and should be removed for cleaner code maintenance.
Apply this diff:
-//if ( defined('SCRIPT_DEBUG') && SCRIPT_DEBUG ) { - $prefix = 'src/'; - $suffix = ''; -/*} else { - $prefix = ''; - $suffix = '.min'; -}*/ +$prefix = 'src/'; +$suffix = '';📝 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.$prefix = 'src/'; $suffix = '';
487-487:
⚠️ Potential issueAdd null checks for departs_from meta
The tour breadcrumb handling needs additional null checks.
Apply this diff:
-$region = get_post_meta( get_the_ID(), 'departs_from', true ); -if ( false !== $region && isset( $region[0] ) ) { +$region = get_post_meta( get_the_ID(), 'departs_from', true ); +if ( ! empty( $region ) && is_array( $region ) && isset( $region[0] ) ) {Committable suggestion skipped: line range outside the PR's diff.
427-427: 💡 Codebase verification
Parameter change in get_post_meta requires attention
The verification reveals two different usages of the
destination_to_accommodationmeta key:
- In
class-frontend.php, it's being changed to usetruewhich returns a single value- In
class-schema.php, it usesfalseto return an array of values, which is then accessed by indexThis change will break the schema implementation as it expects an array of destinations to extract country and region information. The parameter should remain as
falseto maintain compatibility.
includes/classes/legacy/class-frontend.php: Revert the parameter tofalseto maintain consistency with schema usageincludes/classes/legacy/class-schema.php: Update array access logic if the parameter change is intentional🔗 Analysis chain
Verify impact of get_post_meta parameter change
The change from
falsetotruein get_post_meta affects how the destination data is returned. This could impact existing functionality.
Let me analyze the results and generate additional verification to understand the impact of the parameter change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other instances of destination_to_accommodation meta usage echo "Checking meta key usage patterns..." rg "destination_to_accommodation" -A 3 -B 3Length of output: 3482
Script:
#!/bin/bash # Check how the destination_to_accommodation meta is processed in the codebase echo "Checking usage of destination_to_accommodation meta value..." rg "destination_to_accommodation.*true|destination_to_accommodation.*false" -A 5 -B 5 # Check the frontend class implementation for destination handling echo "Checking destination handling in frontend class..." ast-grep --pattern 'function $_($$$) { $$$ get_post_meta($_,"destination_to_accommodation",$_) $$$ }'Length of output: 2461
changelog.md (2)
5-10: 🧹 Nitpick (assertive)
Fix markdown formatting issues
The markdown formatting needs attention:
- Links are using reversed syntax on lines 6-7
- Headings lack proper spacing
- Lists need surrounding blank lines
Apply these corrections:
### Removed - UIX vendor library handling the TO Settings and refactored the settings code - [PR #332](https://github.com/lightspeedwp/tour-operator/pull/332) - Various settings which are now handled by Block and Site Editor options. - [PR #332](https://github.com/lightspeedwp/tour-operator/pull/332) ### Security - General testing to ensure compatibility with latest WordPress version (6.6.1).📝 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.### Removed - UIX vendor library handling the TO Settings and refactored the settings code - [PR #332](https://github.com/lightspeedwp/tour-operator/pull/332) - Various settings which are now handled by Block and Site Editor options. - [PR #332](https://github.com/lightspeedwp/tour-operator/pull/332) ### Security - General testing to ensure compatibility with latest WordPress version (6.6.1).🧰 Tools
🪛 Markdownlint
6-6: (PR #332)[https://github.com//pull/332]
Reversed link syntax(MD011, no-reversed-links)
7-7: (PR #332)[https://github.com//pull/332]
Reversed link syntax(MD011, no-reversed-links)
5-5: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
9-9: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
6-6: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
10-10: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
3-10: 🧹 Nitpick (assertive)
💡 Codebase verification
Documentation and Migration Guide Required for Breaking Changes
The major version upgrade to 2.0.0 introduces significant changes to settings management and WordPress integration, but lacks proper documentation for users to migrate:
- No migration guides found in the repository
- External documentation links in README.md are outdated (still referencing 1.x version paths)
- No upgrade-related classes or functions detected to handle the transition
Please add:
- A detailed migration guide explaining how to move from UIX settings to Block/Site Editor options
- Updated documentation links in README.md reflecting the 2.0.0 changes
- Clear instructions for users about where to find relocated settings
- Examples of using the new Block Editor integration for TO post types
🔗 Analysis chain
Version number suggests breaking changes - verify compatibility
The jump to version 2.0.0 indicates major breaking changes, particularly:
- Removal of UIX vendor library
- Migration of settings to Block and Site Editor options
- WordPress 6.6.1 compatibility updates
Please ensure:
- Migration guides are provided for existing users
- Documentation is updated to reflect new settings locations
- Breaking changes are clearly communicated in release notes
Let me gather more information about documentation and breaking changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for migration guides and documentation updates echo "Checking for migration documentation..." fd -t f -e md -e txt . | xargs rg -i "migrat|upgrade|breaking.?changes"Length of output: 246
Script:
#!/bin/bash # Check for documentation files and README updates echo "Checking documentation files..." fd -t f README.md docs fd -t f -e md . | xargs rg -i "2\.0|block.?editor|site.?editor|settings" # Check for any upgrade/migration related files echo -e "\nChecking for upgrade related files..." fd -t f -e php . | xargs rg -i "class.+upgrade|function.+upgrade"Length of output: 1732
🧰 Tools
🪛 Markdownlint
6-6: (PR #332)[https://github.com//pull/332]
Reversed link syntax(MD011, no-reversed-links)
7-7: (PR #332)[https://github.com//pull/332]
Reversed link syntax(MD011, no-reversed-links)
5-5: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
9-9: Expected: 1; Actual: 0; Below
Headings should be surrounded by blank lines(MD022, blanks-around-headings)
6-6: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
10-10: null
Lists should be surrounded by blank lines(MD032, blanks-around-lists)
includes/classes/legacy/class-tour-operator.php (2)
983-983:
⚠️ Potential issueReconsider commenting out the theme compatibility notice.
The theme compatibility notice has been commented out, but the compatibility check logic still exists. This could leave users without proper notification about theme requirements, potentially leading to setup issues.
Consider either:
- Removing the entire theme compatibility check if it's no longer needed
- Keeping the notice active if theme compatibility is still required
369-371: 💡 Codebase verification
Inconsistent Google Maps API key access patterns found
There is a remaining reference to the old structure
options['api']['googlemaps_key']inincludes/template-tags/maps.phpwhile the new structureoptions['googlemaps_key']is used in other files:
includes/template-tags/maps.php: Uses old structureoptions['api']['googlemaps_key']includes/metaboxes/config-destination.php: Uses new structureoptions['googlemaps_key']includes/metaboxes/config-accommodation.php: Uses new structureoptions['googlemaps_key']includes/classes/legacy/class-tour-operator.php: Uses new structureoptions['googlemaps_key']The
maps.phptemplate tag file needs to be updated to use the new structure to maintain consistency.🔗 Analysis chain
Verify the Google Maps API key access pattern across the codebase.
The change from
$this->options['api']['googlemaps_key']to$this->options['googlemaps_key']could impact other parts of the codebase that might still expect the old structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old API key structure rg "options\['api'\]\['googlemaps_key'\]" # Search for current uses of the new structure rg "options\['googlemaps_key'\]"Length of output: 1020
includes/classes/admin/class-admin.php (2)
33-37:
⚠️ Potential issueSecurity concern: Allowing SVG and KML uploads can pose risks
Allowing SVG and KML file uploads can introduce security vulnerabilities, as these file types may contain malicious code. It's advisable to implement sanitisation for these files or use a secure library to handle SVG and KML uploads safely.
55-62: 🛠️ Refactor suggestion
Sanitise URLs and secure external links
It's best practice to escape URLs using
esc_url()to ensure they're properly sanitised. Additionally, when usingtarget="_blank", it's recommended to addrel="noopener noreferrer"to prevent security vulnerabilities like reverse tabnabbing.Apply the following changes:
- '<a href="' . admin_url( 'admin.php?page=lsx-to-settings' ) . '">' . esc_html__( 'Settings', 'tour-operator' ) . '</a>', + '<a href="' . esc_url( admin_url( 'admin.php?page=lsx-to-settings' ) ) . '">' . esc_html__( 'Settings', 'tour-operator' ) . '</a>', - '<a href="https://www.lsdev.biz/lsx/documentation/lsx-tour-operator/" target="_blank">' . esc_html__( 'Documentation', 'tour-operator' ) . '</a>', + '<a href="' . esc_url( 'https://www.lsdev.biz/lsx/documentation/lsx-tour-operator/' ) . '" target="_blank" rel="noopener noreferrer">' . esc_html__( 'Documentation', 'tour-operator' ) . '</a>', - '<a href="https://www.lsdev.biz/lsx/support/" target="_blank">' . esc_html__( 'Support', 'tour-operator' ) . '</a>', + '<a href="' . esc_url( 'https://www.lsdev.biz/lsx/support/' ) . '" target="_blank" rel="noopener noreferrer">' . esc_html__( 'Support', 'tour-operator' ) . '</a>',📝 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.public function add_action_links( $links ) { $mylinks = array( '<a href="' . esc_url( admin_url( 'admin.php?page=lsx-to-settings' ) ) . '">' . esc_html__( 'Settings', 'tour-operator' ) . '</a>', '<a href="' . esc_url( 'https://www.lsdev.biz/lsx/documentation/lsx-tour-operator/' ) . '" target="_blank" rel="noopener noreferrer">' . esc_html__( 'Documentation', 'tour-operator' ) . '</a>', '<a href="' . esc_url( 'https://www.lsdev.biz/lsx/support/' ) . '" target="_blank" rel="noopener noreferrer">' . esc_html__( 'Support', 'tour-operator' ) . '</a>', ); return array_merge( $links, $mylinks );assets/js/blocks/linked-cover.js (4)
51-54: 🧹 Nitpick (assertive)
Internationalise user-facing strings with
wp.i18n.__.For better localisation support, wrap user-facing strings using internationalisation functions.
First, import
__fromwp.i18n:+const { __ } = wp.i18n;Then update the strings:
createElement( PanelBody, - { title: 'Cover Link Settings', initialOpen: true }, + { title: __('Cover Link Settings', 'text-domain'), initialOpen: true }, createElement(TextControl, { - label: 'Link URL', + label: __('Link URL', 'text-domain'), value: linkUrl, onChange: function (value) { setAttributes({ linkUrl: value }); } }), createElement(ToggleControl, { - label: 'Open link in a new tab', + label: __('Open link in a new tab', 'text-domain'), checked: linkTarget === '_blank', onChange: function (value) { setAttributes({ linkTarget: value ? '_blank' : '_self' }); } }) )Remember to replace
'text-domain'with your plugin's actual text domain.Also applies to: 61-62
83-89:
⚠️ Potential issueAvoid wrapping content in an
<a>tag to prevent accessibility issues.Wrapping the entire cover block in an
<a>tag can cause accessibility and HTML validation problems, especially if the cover block contains interactive elements like buttons or links.Consider alternative approaches:
- Option 1: Use JavaScript to handle click events on the cover block, triggering navigation without altering the HTML structure.
- Option 2: Check if the cover block contains interactive elements before wrapping it in an
<a>tag to avoid nesting interactive content.Would you like assistance in implementing one of these alternatives?
23-25: 🧹 Nitpick (assertive)
Restrict
linkTargetto allowed values usingenum.To prevent invalid values, it's advisable to restrict the
linkTargetattribute to specific allowed options using anenum.Apply this diff to enforce the allowed values:
settings.attributes = Object.assign({}, settings.attributes, { linkUrl: { type: 'string', default: '' }, linkTarget: { type: 'string', default: '_self', // '_self' for same tab, '_blank' for new tab + enum: ['_self', '_blank'] } });📝 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.type: 'string', default: '_self', // '_self' for same tab, '_blank' for new tab enum: ['_self', '_blank'] }
85-88: 🛠️ Refactor suggestion
Include
relattribute for links opening in a new tab to enhance security.When
linkTargetis set to'_blank', it's best practice to addrel="noopener noreferrer"to prevent security vulnerabilities like reverse tabnabbing.Update the
createElementcall to conditionally include therelattribute:return createElement( 'a', - { href: linkUrl, target: linkTarget }, + { + href: linkUrl, + target: linkTarget, + rel: linkTarget === '_blank' ? 'noopener noreferrer' : undefined + }, element );📝 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.'a', { href: linkUrl, target: linkTarget, rel: linkTarget === '_blank' ? 'noopener noreferrer' : undefined }, elementincludes/classes/admin/class-setup.php (2)
165-170: 🛠️ Refactor suggestion
Add error handling for missing configuration files
The
includestatement at line 167 may produce warnings or errors if the configuration file does not exist or is invalid. Consider adding a check to verify if the file exists before including it.Apply the following diff to handle missing files:
public function get_custom_fields( $type = '' ) { $fields = array(); if ( '' !== $type ) { + $config_file = LSX_TO_PATH . 'includes/metaboxes/config-' . $type . '.php'; + if ( file_exists( $config_file ) ) { + $fields = include( $config_file ); + } + } return $fields; }📝 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.$fields = array(); if ( '' !== $type ) { $config_file = LSX_TO_PATH . 'includes/metaboxes/config-' . $type . '.php'; if ( file_exists( $config_file ) ) { $fields = include( $config_file ); } } return $fields; }
137-139: 🧹 Nitpick (assertive)
Adjust array alignment for consistency with coding standards
The static analysis tool reports misalignment of array double arrows on lines 137 and 139. Please adjust the spacing to comply with the WordPress coding standards.
Apply the following diff to fix the alignment:
'id' => 'lsx_to_metabox_' . $post_type . '_' . $metabox_counter, -'title' => $field['name'], +'title' => $field['name'], 'object_types' => array( $post_type ), // Post type -'context' => 'normal', +'context' => 'normal', 'priority' => 'high', 'show_names' => true,📝 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.'id' => 'lsx_to_metabox_' . $post_type . '_' . $metabox_counter, 'title' => $field['name'], 'object_types' => array( $post_type ), // Post type 'context' => 'normal', 'priority' => 'high', 'show_names' => true,🧰 Tools
🪛 GitHub Check: CodeFactor
[notice] 139-139: includes/classes/admin/class-setup.php#L139
Array double arrow not aligned correctly; expected 6 space(s) between "'context'" and double arrow, but found 7. (WordPress.Arrays.MultipleStatementAlignment-[fixable])
[notice] 137-137: includes/classes/admin/class-setup.php#L137
Array double arrow not aligned correctly; expected 8 space(s) between "'title'" and double arrow, but found 9. (WordPress.Arrays.MultipleStatementAlignment-[fixable])assets/css/style.css (3)
5-7: 🛠️ Refactor suggestion
Consolidate conflicting CSS declarations for
.currency-icon:after.The selector
.lsx-price-wrapper .amount .currency-icon:afteris declared twice (lines 5-7 and 92-93) with conflictingmargin-rightproperties. This can cause confusion and unexpected styling. Please consolidate these declarations to avoid conflicts.Apply this diff to merge the declarations:
-.lsx-price-wrapper .amount .currency-icon:after { - margin-left: 3px; - margin-right: 3px; -} -.lsx-price-wrapper .amount .currency-icon:after { - margin-right: 0px; + .lsx-price-wrapper .amount .currency-icon:after { + margin-left: 3px; + margin-right: 0px; }Also applies to: 92-93
76-86: 🧹 Nitpick (assertive)
Consider removing commented-out code for cleanliness.
There are blocks of commented-out code between lines 76-86 and 88-90. If these styles are no longer needed, consider removing them to improve the readability and maintainability of the stylesheet.
Also applies to: 88-90
33-34:
⚠️ Potential issueCorrect the currency symbol for Indonesian Rupiah (IDR).
The currency symbol for 'idr' (Indonesian Rupiah) is incorrectly set to '₹', which is the symbol for the Indian Rupee. The correct symbol for the Indonesian Rupiah is 'Rp'.
Apply this diff to fix the currency symbol:
-.lsx-price-wrapper .amount .currency-icon.idr:after { - content: "₹"; +.lsx-price-wrapper .amount .currency-icon.idr:after { + content: "Rp"; }Committable suggestion skipped: line range outside the PR's diff.
includes/classes/legacy/class-accommodation.php (1)
68-74:
⚠️ Potential issueUsing an empty string as an array key may lead to unintended behaviour
Using an empty string (
'') as an array key can cause unexpected behaviour when accessing or manipulating the array. Some functions may not handle empty keys as expected, which might lead to unexpected results. It's advisable to use a more explicit key, such as'none', to prevent potential issues.Apply this diff to fix the issue:
- '' => esc_html__( 'None', 'tour-operator' ), + 'none' => esc_html__( 'None', 'tour-operator' ),📝 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.'none' => esc_html__( 'None', 'tour-operator' ), 'chalet' => esc_html__( 'Chalet', 'tour-operator' ), 'room' => esc_html__( 'Room', 'tour-operator' ), 'spa' => esc_html__( 'Spa', 'tour-operator' ), 'tent' => esc_html__( 'Tent', 'tour-operator' ), 'villa' => esc_html__( 'Villa', 'tour-operator' ), );includes/classes/legacy/class-unit-query.php (3)
177-179: 🧹 Nitpick (assertive)
Correct the method docblock to reflect 'price' instead of 'description'
The docblock for
item_priceincorrectly mentions 'description'. Please update it to 'Outputs the current item's "price" field' to accurately describe the method.
192-194: 🧹 Nitpick (assertive)
Correct the method docblock to reflect 'type' instead of 'description'
The docblock for
item_typemistakenly references 'description'. Kindly update it to 'Outputs the current item's "type" field' for clarity and correctness.
87-87:
⚠️ Potential issuePotential issue: get_post_meta may not return an array
Using
get_post_meta( \$this->post_id, 'units', true )retrieves a single value, but later the code expects $this->queried_items to be an array. This may lead to errors when $this->queried_items is not an array.Consider setting the third parameter to
falseto ensure an array is returned:- $this->queried_items = get_post_meta( $this->post_id, 'units', true ); + $this->queried_items = get_post_meta( $this->post_id, 'units', false );📝 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.$this->queried_items = get_post_meta( $this->post_id, 'units', false );includes/classes/blocks/class-registration.php (6)
97-209: 🧹 Nitpick (assertive)
Refactor to reduce code duplication
Multiple cases within the
switchstatement contain similar logic, particularly for processing related items. This repetition can make maintenance more challenging and increase the risk of errors.Consider abstracting the repeated logic into private helper methods. This will improve code maintainability and readability.
143-154: 🧹 Nitpick (assertive)
Initialize
$excluded_itemsas an arrayThere is a possibility that
$excluded_itemscould befalseornullif the meta key doesn't exist, which could lead to issues when using it later in the code.Initialize
$excluded_itemsas an empty array before the conditional checks to ensure it's always an array.
196-197: 🧹 Nitpick (assertive)
Simplify the check for
$found_itemsThe check
if ( false !== $found_items && ! empty( $found_items ) )is unnecessary becauseget_post_metawill return an empty string or array, notfalse, when the meta key doesn't exist.Simplify the condition to:
if ( ! empty( $found_items ) ) {This makes the code clearer and avoids redundant checks.
52-65: 🧹 Nitpick (assertive)
Optimize script dependencies for better performance
Review the dependencies listed when enqueuing
linked-cover.jsandslider-query.js. Some dependencies might not be necessary, which can affect performance.Verify and remove any unused dependencies to streamline script loading.
121-123:
⚠️ Potential issueEnsure correct meta value comparison
In the meta query, comparing the
'featured'meta key totruemay not yield the expected results, as meta values are stored as strings in the database. Comparing a string to a boolean can cause issues.Update the comparison to match the stored meta value, such as
'1'or'true', depending on how it's saved.Apply this diff to correct the comparison:
'key' => 'featured', -'value' => true, +'value' => '1', 'compare' => '=',📝 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.'value' => '1', 'compare' => '=', ),
39-39: 🛠️ Refactor suggestion
Ensure scripts are enqueued only in the block editor
The
is_admin()function returnstruefor all admin pages, including non-editor pages. To enqueue scripts exclusively in the block editor, consider usingif ( function_exists( 'register_block_type' ) && is_admin() ). This ensures scripts are not unnecessarily loaded on other admin pages.Apply this diff to improve script loading:
-if ( is_admin() && function_exists( 'register_block_type' ) ) { +if ( function_exists( 'register_block_type' ) && is_admin() ) {📝 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.if ( function_exists( 'register_block_type' ) && is_admin() ) {includes/classes/legacy/class-tour.php (1)
187-189: 🧹 Nitpick (assertive)
Fix the spacing issue in the if condition
The static analysis tool has identified an extra space before the closing bracket on line 187. Removing this unnecessary space improves code readability and adheres to coding standards.
Apply this diff to correct the spacing:
- if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] ) ) { + if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] )) {📝 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.if ( is_object( $tour_operator ) && isset( $tour_operator->options['currency'] ) && ! empty( $tour_operator->options['currency'] )) { $currency = $tour_operator->options['currency']; $currency = '<span class="currency-icon ' . strtolower( $currency ) . '">' . $currency . '</span>';🧰 Tools
🪛 GitHub Check: CodeFactor
[warning] 187-187: includes/classes/legacy/class-tour.php#L187
Expected 0 spaces before closing bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable])includes/classes/legacy/class-admin.php (4)
29-34: 🧹 Nitpick (assertive)
Consider initialising the new public properties
The public properties
$connections,$single_fields, and$taxonomiesare declared but not initialised. Initialising them to an empty array or appropriate default value can improve code clarity and prevent potential warnings.
280-299:
⚠️ Potential issuePerform nonce verification before accessing
$_POSTvariablesSecurity best practice recommends verifying nonces before processing any user input. Move the
check_admin_referer()call before accessing$_POST['thumbnail']and$_POST['tagline']to ensure the request is valid.Apply this diff to reorder the code:
-if ( ! isset( $_POST['thumbnail'] ) || ! isset( $_POST['tagline'] ) ) { - return; -} -if ( check_admin_referer( 'lsx_to_save_term_thumbnail', 'lsx_to_term_thumbnail_nonce' ) ) { +if ( ! check_admin_referer( 'lsx_to_save_term_thumbnail', 'lsx_to_term_thumbnail_nonce' ) ) { + return; +} +if ( ! isset( $_POST['thumbnail'] ) || ! isset( $_POST['tagline'] ) ) { return; }Committable suggestion skipped: line range outside the PR's diff.
316-319:
⚠️ Potential issueEscape user input when rendering form fields
In the
add_tagline_form_field()function, replacewp_kses_post()withesc_attr()when outputting the$valuein the input field to properly escape HTML attributes.Apply this diff:
-<input name="tagline" id="tagline" type="text" value="<?php echo wp_kses_post( $value ); ?>" size="40"> +<input name="tagline" id="tagline" type="text" value="<?php echo esc_attr( $value ); ?>" size="40">📝 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.<tr class="form-field term-tagline-wrap"> <th scope="row"><label for="tagline"><?php esc_html_e( 'Tagline', 'tour-operator' ); ?></label></th> <td> <input name="tagline" id="tagline" type="text" value="<?php echo esc_attr( $value ); ?>" size="40">
247-255:
⚠️ Potential issueSanitise and escape variables when outputting HTML attributes
When outputting
$valueand$image_previewin your HTML, it's important to use the correct escaping functions to prevent XSS vulnerabilities. Replacewp_kses_post()withesc_attr()oresc_url()as appropriate.Apply this diff to enhance security:
-<input class="input_image" type="hidden" name="thumbnail" value="<?php echo wp_kses_post( $value ); ?>"> +<input class="input_image" type="hidden" name="thumbnail" value="<?php echo esc_attr( $value ); ?>"> ... -<?php echo wp_kses_post( $image_preview ); ?> +<?php echo $image_preview; ?>Ensure that
$image_previewis safely constructed using appropriate sanitisation functions when building the image tag.Committable suggestion skipped: line range outside the PR's diff.
assets/js/blocks/general.js (7)
24-38: 🧹 Nitpick (assertive)
Avoid using hardcoded absolute URLs for placeholder images
The URLs for placeholder images are hardcoded with absolute paths pointing to
https://tour-operator.lsx.design. This might lead to issues in different environments or if the domain changes. Consider using relative URLs or WordPress functions likeplugin_dir_urlto generate the paths dynamically.
1-431: 🧹 Nitpick (assertive)
Consider externalising repeated code into reusable functions
There is a significant amount of repeated code across the block variations, particularly in setting styles and attributes. Refactoring this into reusable functions or constants could improve maintainability and reduce potential errors.
408-410:
⚠️ Potential issueRemove duplicate 'name' property and correct the variation configuration
There is a duplicate
nameproperty in the block variation definition, which can cause unexpected behaviour. The secondname: 'core/button'is incorrect and overrides the intended variation name.Apply this diff to fix the issue:
wp.blocks.registerBlockVariation('core/button', { name: 'lsx-tour-operator/more-link', title: 'More Button', - name: 'core/button',📝 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.name: 'lsx-tour-operator/more-link', title: 'More Button',
248-250:
⚠️ Potential issueReplace localhost URLs with valid image paths
The image URL at line 248 is set to
http://localhost:8883, which is a development URL and won't work in a live environment. This will cause the image not to display for users. Please update the URL to a valid path accessible in production.Apply this diff to fix the issue:
- url: 'http://localhost:8883/wp-content/themes/lsx-tour-operator/assets/images/best-months-to-travel-TO-icon-black-20px-1-1.png', + url: '/wp-content/themes/lsx-tour-operator/assets/images/best-months-to-travel-TO-icon-black-20px-1-1.png',📝 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.url: '/wp-content/themes/lsx-tour-operator/assets/images/best-months-to-travel-TO-icon-black-20px-1-1.png', alt: '' }],
143-145:
⚠️ Potential issueReplace localhost URLs with valid image paths
The image URL at line 143 points to
http://localhost:8883, which won't be accessible in production environments. This will result in broken images for end-users. Please replace it with a valid URL or use a relative path.Apply this diff to fix the issue:
- url: 'http://localhost:8883/wp-content/themes/lsx-tour-operator/assets/images/Typetype-icon.png', + url: '/wp-content/themes/lsx-tour-operator/assets/images/Typetype-icon.png',📝 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.url: '/wp-content/themes/lsx-tour-operator/assets/images/Typetype-icon.png', alt: '' }],
44-47: 🧹 Nitpick (assertive)
Add 'category' attribute for consistent block organisation
The "Price" block variation lacks the
categoryattribute, unlike other variations in the code. Adding thecategoryensures the block is grouped appropriately in the editor.Apply this diff to include the category:
wp.blocks.registerBlockVariation("core/group", { name: "lsx-tour-operator/price", title: "Price", icon: "bank", + category: "lsx-tour-operator",Committable suggestion skipped: line range outside the PR's diff.
418-425:
⚠️ Potential issueCorrect the border radius values for proper styling
The
border.radiusproperties have values that are not valid CSS border radius specifications. ThetopLeftandtopRightproperties have multiple values, which is incorrect. Border radius for each corner should be a single value or use the shorthand property.Apply this diff to fix the issue:
- border: { - radius: { - topLeft: '0px 8px 8px 0px', - topRight: '0px 8px 8px 0px', - bottomLeft: '8px', - bottomRight: '8px' - } - } + style: { + border: { + radius: '0px 8px 8px 0px' + } + },This change uses the correct shorthand notation for
border.radius.Committable suggestion skipped: line range outside the PR's diff.
assets/js/src/custom.js (7)
49-67: 🧹 Nitpick (assertive)
Optimise the click handler in
set_read_moreto prevent unnecessary hide/showIn the click event handler of
lsx_to.set_read_more, the button is hidden with$( this ).hide();and then shown again with$( this ).show();. This might cause unnecessary reflows and potential flicker. Consider removing these calls or adjusting the logic to avoid this redundancy.
111-111: 🛠️ Refactor suggestion
Localise hardcoded strings for internationalisation
The hardcoded strings such as
'View Less'should be localised to support multiple languages, enhancing the user experience for non-English speakers.Apply this diff assuming a localisation function
lsx_to_localizeis available:- button.find('a').text( 'View Less' ); + button.find('a').text( lsx_to_localize( 'View Less' ) );Also applies to: 165-165
256-256:
⚠️ Potential issueRemove debugging statement
console.log($this);The
console.log($this);statement appears to be leftover from debugging. Console logs should be removed from production code to prevent unnecessary logging and potential exposure of sensitive information.Apply this diff to remove the console log:
- console.log($this);📝 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.
141-142: 🧹 Nitpick (assertive)
Use
constinstead ofvarfor variable declarationsSimilarly, in
readMoreSetTI, consider changingvar limit = 1;toconst limit = 1;for consistency and adherence to modern JavaScript best practices.Apply this diff to update the variable declaration:
- var limit = 1; + const limit = 1;📝 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.const limit = 1; let counter = 0;
136-139: 🧹 Nitpick (assertive)
Ensure empty
<p>tags with whitespace are removedWhen removing empty paragraph tags, the current check
'' === $(this).html()may not remove<p>tags that contain only whitespace. Consider trimming the HTML content before comparison to ensure all empty or whitespace-only<p>tags are removed.Apply this diff to improve the condition:
- if ( '' === $(this).html() ) { + if ( '' === $(this).html().trim() ) {📝 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.if ( '' === $(this).html().trim() ) { $(this).remove(); } });
73-74: 🧹 Nitpick (assertive)
Use
constinstead ofvarfor variable declarationsIn
readMoreSet, it's recommended to useconstinstead ofvarfor variables that do not change value. This improves code clarity and prevents potential scoping issues.Apply this diff to update the variable declaration:
- var limit = 1; + const limit = 1;📝 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.const limit = 1; let counter = 0;
365-365: 🧹 Nitpick (assertive)
💡 Codebase verification
Remove the commented out
set_read_more_itineraryfunction callThe itinerary functionality with "view more" links and hidden items is still actively used in the codebase as evidenced by:
- The presence of
class-itinerary-query.phpand itinerary templates- The markup in
includes/layout.phpshowing itinerary items- The "view more" link structure in
includes/functions.phpSince this feature is still in use, the function call should be uncommented to ensure the itinerary read-more functionality works properly.
🔗 Analysis chain
Consider whether to remove or reinstate
set_read_more_itineraryThe function call
lsx_to.set_read_more_itinerary();is commented out. If the itinerary read more functionality is no longer required, consider removing this code for cleanliness. If it is needed, you may want to uncomment it to ensure the feature is active.Run the following script to check if
lsx_to.set_read_more_itineraryis defined and used elsewhere:
Let me analyze the results. I can see that:
- The function
set_read_more_itineraryis defined inassets/js/src/custom.jsand handles showing hidden itinerary items when clicking a "view more" link- The function is called in the jQuery document.ready handler in the minified file
assets/js/custom.min.js- The commented out line in question is in
assets/js/src/custom.jsLet me check if there are any "view more" links in itinerary sections that would need this functionality:
Let me try a different search approach to find any itinerary-related templates or HTML files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for the definition and usage of 'lsx_to.set_read_more_itinerary' echo "Searching for the function definition of 'lsx_to.set_read_more_itinerary':" rg -A 2 $'lsx_to\.set_read_more_itinerary\s*=\s*function' echo "Searching for usages of 'lsx_to.set_read_more_itinerary':" rg $'lsx_to\.set_read_more_itinerary\s*\('Length of output: 18023
Script:
#!/bin/bash # Search for view-more links in itinerary sections rg -l 'id="itinerary".*view-more' --type html rg -l 'class=".*itinerary.*view-more' --type html # Search for itinerary-item elements with hidden class rg -l 'class=".*itinerary-item.*hidden' --type htmlLength of output: 153
Script:
#!/bin/bash # Search for any files containing itinerary-related patterns echo "Searching for itinerary templates:" fd -e php -e html . | grep -i 'itinerary' echo -e "\nSearching for itinerary markup in PHP/HTML files:" rg -t php -t html 'itinerary-item|view-more' echo -e "\nChecking if the itinerary feature is used in templates:" rg -t php "get_template_part.*itinerary"Length of output: 794
includes/functions.php (3)
232-234: 🧹 Nitpick (assertive)
Refactor repeated array checks into a helper function
The pattern of checking if a variable is an array and converting it if not is repeated in multiple places. Consider refactoring this into a utility function to enhance code reusability and maintainability.
Apply this diff to create a helper function and update the code:
+function lsx_to_array( $value ) { + return is_array( $value ) ? $value : array( $value ); +}Then replace the checks in the affected functions:
- if ( is_string( $tour_itinerary->itinerary[ $meta_key ] ) ) { - $tour_itinerary->itinerary[ $meta_key ] = array( $tour_itinerary->itinerary[ $meta_key ] ); - } + $tour_itinerary->itinerary[ $meta_key ] = lsx_to_array( $tour_itinerary->itinerary[ $meta_key ] );- if ( ! is_array( $itinerary_accommodation ) ) { - $itinerary_accommodation = array( $itinerary_accommodation ); - } + $itinerary_accommodation = lsx_to_array( $itinerary_accommodation );Also applies to: 328-329, 353-354
347-361: 🧹 Nitpick (assertive)
Enhance function to handle multiple accommodations
Currently,
lsx_to_itinerary_accommodation_typeonly retrieves the accommodation type of the first accommodation. Consider extending the function to handle multiple accommodations by looping through the array and collecting all associated accommodation types.Implementing this change would provide a more comprehensive output when multiple accommodations are linked.
347-361:
⚠️ Potential issueCheck for empty accommodation array before accessing elements
To prevent potential errors, ensure that
$itinerary_accommodationis not empty before accessing its elements. This will avoid undefined offset notices when the array is empty.Apply this diff to add the necessary check:
if ( ! is_array( $itinerary_accommodation ) ) { $itinerary_accommodation = array( $itinerary_accommodation ); } +if ( ! empty( $itinerary_accommodation ) ) { $return = get_the_term_list( $itinerary_accommodation[0], 'accommodation-type', $before, ', ', $after ); if ( true === $echo ) { echo wp_kses_post( $return ); } +}📝 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.function lsx_to_itinerary_accommodation_type( $before = '', $after = '', $echo = true ) { global $tour_itinerary; $return = ''; if ( $tour_itinerary && $tour_itinerary->has_itinerary && ! empty( $tour_itinerary->itinerary ) && ! empty( $tour_itinerary->itinerary['accommodation_to_tour'] ) ) { $itinerary_accommodation = $tour_itinerary->itinerary['accommodation_to_tour']; if ( ! is_array( $itinerary_accommodation ) ) { $itinerary_accommodation = array( $itinerary_accommodation ); } if ( ! empty( $itinerary_accommodation ) ) { $return = get_the_term_list( $itinerary_accommodation[0], 'accommodation-type', $before, ', ', $after ); if ( true === $echo ) { echo wp_kses_post( $return ); } } } return $return;includes/classes/admin/class-settings.php (10)
82-85: 🧹 Nitpick (assertive)
Confirm that 'settings-fields.php' returns the expected array
If
settings-fields.phpdoes not return an array,$settingsmay not contain the necessary settings data. Ensure that this file returns the settings array as expected, and consider usingrequireif inclusion is essential.
118-118: 🧹 Nitpick (assertive)
Remove commented-out code for clarity
The line
//tour_operator()->legacy->get_post_types();appears to be unused. Removing commented-out code can enhance readability and maintainability.
209-214: 🧹 Nitpick (assertive)
Update docblock to reflect 'currency_settings' function purpose
The docblock mentions "outputs the display settings for the map tab," which does not correspond with the
currency_settingsfunction. Please update the description to accurately reflect that this function handles currency settings.
222-227: 🧹 Nitpick (assertive)
Update docblock to reflect 'map_settings' function purpose
The docblock in
map_settingsrefers to "outputs the display settings for the map tab." Ensure the description accurately reflects the function's role in handling map settings.
235-241: 🧹 Nitpick (assertive)
Provide a docblock for 'fusion_table_settings' function
The
fusion_table_settingsfunction lacks a docblock. Adding a descriptive docblock enhances code readability and understanding.
250-255: 🧹 Nitpick (assertive)
Update docblock to reflect 'map_placeholder_settings' function purpose
The docblock mentions "outputs the display settings for the map tab," which may not accurately describe the
map_placeholder_settingsfunction. Please revise the description to match the function's actual purpose.
470-483: 🛠️ Refactor suggestion
Prevent undefined index error when accessing '$image[0]'
If
$image_idis zero or invalid,$image[0]may be undefined, causing a PHP notice. Ensure$imageis a valid array before accessing$image[0].Consider modifying the code as follows:
if ( $image_id ) { $image = wp_get_attachment_image_src( $image_id, 'medium' ); $image_src = $image[0]; $prev_css = ''; } else { $image_src = ''; $prev_css = 'display:none;'; } $field[] = '<div class="thumbnail-preview" style="' . $prev_css . '"><img src="' . $image_src . '" width="' . $params['preview_w'] . '" style="color:black;" /></div>';
310-311:
⚠️ Potential issueCorrect syntax error in 'default' case of switch statement
The 'default' case in the switch statement should use a colon ':' instead of a semicolon ';'. This ensures the 'default' case is correctly handled by PHP.
Apply this diff to fix the syntax error:
- default; + default:📝 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.default: }
345-345:
⚠️ Potential issueRemove invalid 'type' attribute from '' tag The <select> HTML tag does not support a 'type' attribute. Including it may result in invalid HTML. Please remove the 'type' attribute from the <select> tag. Apply this diff to correct the HTML: - $field[] = '<select type="' . $params['type'] . '" name="' . $field_id . '" />'; + $field[] = '<select name="' . $field_id . '">'; 📝 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. $field[] = '<select name="' . $field_id . '">'; 568-569:⚠️ Potential issue Sanitize redirect URL when using 'wp_safe_redirect' For security reasons, it's important to sanitize the redirect URL. Use esc_url_raw() to ensure the URL is safe before redirecting. Apply this diff to address the issue: - wp_safe_redirect( $_POST[ '_wp_http_referer' ] ); + wp_safe_redirect( esc_url_raw( $_POST[ '_wp_http_referer' ] ) ); 📝 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. wp_safe_redirect( esc_url_raw( $_POST[ '_wp_http_referer' ] ) ); exit; assets/js/blocks/tour.js (5) 406-542: 🧹 Nitpick (assertive) Refactor to reduce code duplication among block variations The block variations for 'Duration', 'Group Size', and 'Single Supplement' have similar structures. Consider creating a factory function or reusable component to generate these variations, which will enhance maintainability and reduce redundancy. 147-154: 🧹 Nitpick (assertive) Avoid hardcoding image URLs; use dynamic image sources Hardcoding image URLs can cause issues when moving between environments or if the URLs change. Consider using attachment IDs or allowing users to select the image within the editor. Apply this diff to use the image's attachment ID: - url: "https://tour-operator.lsx.design/wp-content/uploads/2024/09/wetu-map-figme-prototype-image.png", + id: 1234, // Replace with the actual media ID of the image Committable suggestion skipped: line range outside the PR's diff. 578-584: 🧹 Nitpick (assertive) Avoid hardcoding image IDs; consider dynamic image selection Hardcoding the image 'id' can cause discrepancies across environments since media IDs may vary. Consider allowing users to select the image or using a dynamic method to retrieve it. Apply this diff to remove the hardcoded 'id': - id: 122731, + // Allow users to select the image or provide a dynamic default Committable suggestion skipped: line range outside the PR's diff. 327-334: 🧹 Nitpick (assertive) Avoid hardcoding image URLs; use dynamic image sources Using hardcoded image URLs may lead to issues across different environments. It's better to use the image's attachment ID or allow users to select the image dynamically. Apply this diff: - url: 'https://tour-operator.lsx.design/wp-content/uploads/2024/09/Typelocation-icon.png', + id: 1235, // Replace with the actual media ID Committable suggestion skipped: line range outside the PR's diff. 163-291:⚠️ Potential issue Correct the function call to 'wp.blocks.registerBlockVariation' The function 'wp.registerBlockVariation' is incorrect. You should use 'wp.blocks.registerBlockVariation' to properly register the block variation. Apply this diff to fix the issue: - wp.registerBlockVariation( 'core/group', { + wp.blocks.registerBlockVariation( 'core/group', { 📝 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. wp.blocks.registerBlockVariation( 'core/group', { name: 'price-include-exclude', title: 'Price Include & Exclude', category: 'layout', attributes: { align: 'wide', className: 'lsx-include-exclude-wrapper', style: { spacing: { padding: { top: 'var:preset|spacing|x-small', bottom: 'var:preset|spacing|x-small', left: 'var:preset|spacing|x-small', right: 'var:preset|spacing|x-small' } }, border: { radius: '8px', width: '1px' } }, backgroundColor: 'base', borderColor: 'primary', layout: { type: 'constrained' } }, innerBlocks: [ [ 'core/columns', { align: 'wide', style: { spacing: { blockGap: { top: 'var:preset|spacing|medium', left: 'var:preset|spacing|medium' } } } }, [ [ 'core/column', { width: '50%', id: 'lsx-included-wrapper', style: { spacing: { blockGap: '0' } } }, [ [ 'core/paragraph', { style: { elements: { link: { color: { text: 'var:preset|color|primary-700' } } } }, textColor: 'primary-700', fontSize: 'medium' }, [ 'Price Includes:' ] ], [ 'core/paragraph', { metadata: { bindings: { content: { source: 'lsx/post-meta', args: { key: 'included' } } } } }, [] ] ] ], [ 'core/column', { width: '50%', className: 'lsx-not-included-wrapper', style: { spacing: { blockGap: '0' } } }, [ [ 'core/paragraph', { style: { elements: { link: { color: { text: 'var:preset|color|primary-700' } } } }, textColor: 'primary-700', fontSize: 'medium' }, [ 'Price Excludes:' ] ], [ 'core/paragraph', { metadata: { bindings: { content: { source: 'lsx/post-meta', args: { key: 'not_included' } } } } }, [] ] ] ] ] ] ] } ); includes/classes/blocks/class-bindings.php (13) 375-377:⚠️ Potential issue Ensure variable '$rooms' is defined In the build_image method, the variable $rooms is used but may not be defined in the current scope. Ensure that $rooms is properly defined or initialized before use to avoid undefined variable errors. 237-237: 🧹 Nitpick (assertive) Remove trailing whitespace There is unnecessary whitespace at the end of line 237. Remove the trailing whitespace to maintain code cleanliness. 🧰 Tools 🪛 GitHub Check: CodeFactor [warning] 237-237: includes/classes/blocks/class-bindings.php#L237 Whitespace found at end of line. (Squiz.WhiteSpace.SuperfluousWhitespace-[fixable]) 782-793: 🧹 Nitpick (assertive) Optimize loop for checking meta values The loop for checking meta values can be optimized for efficiency. Consider short-circuiting the loop when a value is found: foreach ( $key_array as $meta_key ) { $value = lsx_to_custom_field_query( $meta_key, '', '', false ); if ( ! empty( $value ) ) { $has_values = true; break; } } 🧰 Tools 🪛 GitHub Check: CodeFactor [warning] 783-783: includes/classes/blocks/class-bindings.php#L783 Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable]) 346-346: 🧹 Nitpick (assertive) Correct typographical errors in comment The comment on line 346 contains typos. It should read 'empty' instead of 'emtpy', and 'then' instead of 'than'. Apply this diff to fix the comment: - // if the value is emtpy than add a css class to hide the element. + // If the value is empty, then add a CSS class to hide the element. Committable suggestion skipped: line range outside the PR's diff. 346-346: 🧹 Nitpick (assertive) Optimize empty value check The condition checks if $value is empty using multiple methods. This can be simplified. Simplify the condition: - if ( '' === $value || false === $value || empty( $value ) ) { + if ( empty( $value ) ) { Since empty( $value ) will cover cases where $value is '' or false. 📝 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. if ( empty( $value ) ) { 170-170: 🧹 Nitpick (assertive) Remove unnecessary space after opening bracket There is an extra space after the opening parenthesis in the if statement. Apply this diff to correct the spacing: - if ( true === $only_parents ) { + if ( true === $only_parents ) { Committable suggestion skipped: line range outside the PR's diff. 🧰 Tools 🪛 GitHub Check: CodeFactor [warning] 170-170: includes/classes/blocks/class-bindings.php#L170 Expected 0 spaces after opening bracket; 1 found. (Squiz.WhiteSpace.ControlStructureSpacing-[fixable]) 693-693: 🧹 Nitpick (assertive) Remove extra space before closing parenthesis The if statement on line 693 has two spaces before the closing parenthesis; it should have only one. Apply this diff: - if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) { + if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) { 📝 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. if ( ! isset( $parsed_block['blockName'] ) || ! isset( $parsed_block['attrs'] ) ) { 🧰 Tools 🪛 GitHub Check: CodeFactor [warning] 693-693: includes/classes/blocks/class-bindings.php#L693 Expected exactly one space before closing parenthesis; " " found. (WordPress.WhiteSpace.ControlStructureSpacing-[fixable]) 71-71:⚠️ Potential issue Typographical error in method name 'maybe_hide_varitaion' There's a typo in the method name 'maybe_hide_varitaion' used in the add_filter call. It should be 'maybe_hide_variation'. Apply this diff to correct the typo: - add_filter( 'render_block', array( $this, 'maybe_hide_varitaion' ), 10, 3 ); + add_filter( 'render_block', array( $this, 'maybe_hide_variation' ), 10, 3 ); Ensure that the method definition is also updated accordingly. 📝 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. add_filter( 'render_block', array( $this, 'maybe_hide_variation' ), 10, 3 ); 691-691:⚠️ Potential issue Typographical error in method name 'maybe_hide_varitaion' The method maybe_hide_varitaion is misspelled. It should be maybe_hide_variation. Apply this diff to correct the method name: - public function maybe_hide_varitaion( $block_content, $parsed_block, $block_obj ) { + public function maybe_hide_variation( $block_content, $parsed_block, $block_obj ) { Remember to update all references to this method throughout the code. 📝 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. public function maybe_hide_variation( $block_content, $parsed_block, $block_obj ) { 551-551: 🧹 Nitpick (assertive) Parameter '$block_obj' is unused in 'render_gallery_block' The parameter $block_obj in the render_gallery_block method is not used within the method body. Consider removing the unused parameter to clean up the method signature: - public function render_gallery_block( $block_content, $parsed_block, $block_obj ) { + public function render_gallery_block( $block_content, $parsed_block ) { Ensure that this change does not affect overriding or interfacing methods. 📝 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. public function render_gallery_block( $block_content, $parsed_block ) { 🧰 Tools 🪛 GitHub Check: CodeFactor [notice] 551-551: includes/classes/blocks/class-bindings.php#L551 The method parameter $block_obj is never used. (Generic.CodeAnalysis.UnusedFunctionParameter) 551-551: 🧹 Nitpick (assertive) Add return type documentation The method render_gallery_block does not document its return type. Add a @return annotation to the doc block to specify the return type. /** * Renders the gallery block with custom content. + * + * @param string $block_content The original content of the block. + * @param array $parsed_block Parsed data for the block. + * @return string The modified block content. */ Committable suggestion skipped: line range outside the PR's diff. 🧰 Tools 🪛 GitHub Check: CodeFactor [notice] 551-551: includes/classes/blocks/class-bindings.php#L551 The method parameter $block_obj is never used. (Generic.CodeAnalysis.UnusedFunctionParameter) 98-98: 🧹 Nitpick (assertive) Add trailing comma in multiline array For consistency and to prevent syntax errors when extending arrays, it's recommended to add a trailing comma after the last item in a multiline array. Apply this diff: 'get_value_callback' => array( $this, 'empty_callback' ) + , Committable suggestion skipped: line range outside the PR's diff. 🧰 Tools 🪛 GitHub Check: CodeFactor [warning] 98-98: includes/classes/blocks/class-bindings.php#L98 There should be a comma after the last array item in a multi-line array. (NormalizedArrays.Arrays.CommaAfterLast-[fixable]) 225-225: 🧹 Nitpick (assertive) Missing doc comment for parameter '$block_obj' The method post_meta_callback lacks a doc comment for the $block_obj parameter. Add the missing parameter description to the doc block. /** * Processes post meta data for block bindings. * * @param array $source_args Source arguments. * @param object $block_instance Block instance. + * @param object $block_obj Block object. * @return mixed */ Committable suggestion skipped: line range outside the PR's diff. assets/js/blocks/accommodation.js (2) 311-312:⚠️ Potential issue Correct the typo in 'name' attribute In the 'Number of Rooms' block variation, the 'name' attribute has a typo: it should be 'Number of Rooms' instead of 'Number if Rooms'. Apply this diff to fix the typo: - name: 'Number if Rooms', + name: 'Number of Rooms', 📝 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. name: 'Number of Rooms', }, 846-846:⚠️ Potential issue Fix the block variation 'name' to ensure consistency The block variation 'name' is incorrectly specified as 'llsx-tour-operator/suggested-visitor-types'. It should be 'lsx-tour-operator/suggested-visitor-types' to maintain consistency with other block variations. Apply this diff to correct the 'name' attribute: - name: 'llsx-tour-operator/suggested-visitor-types', + name: 'lsx-tour-operator/suggested-visitor-types', 📝 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. name: 'lsx-tour-operator/suggested-visitor-types', assets/js/blocks/query-loops.js (8) 3-1094: 🧹 Nitpick (assertive) Consider refactoring to reduce code duplication The code registers multiple block variations with similar structures and attributes. To enhance maintainability and reduce potential errors, consider abstracting the common code into reusable functions or data structures and generate the variations dynamically. 387-387: 🧹 Nitpick (assertive) Typographical error in the comment There's a minor typo in the comment. Change "Should displays" to "Should display" for clarity. -// Should displays accommodation in the same "destinations" as the tour. +// Should display accommodation in the same "destinations" as the tour. 📝 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. // Should display accommodation in the same "destinations" as the tour. 1027-1028:⚠️ Potential issue Correct the 'name' in metadata for 'Related Reviews - Accommodation' The name property in metadata is set to 'Related Reviews - Tour', but it should be 'Related Reviews - Accommodation' to match the title and description. name: 'Related Reviews - Tour' +name: 'Related Reviews - Accommodation' Committable suggestion skipped: line range outside the PR's diff. 203-209:⚠️ Potential issue Add a query filter to fetch only 'Featured' accommodation The 'Featured Accommodation' block is intended to display accommodations tagged as 'Featured', but the query does not filter for these posts. Please include a taxQuery to filter posts with the 'Featured' tag. query: { perPage: 8, postType: 'accommodation', + taxQuery: [ + { + taxonomy: 'post_tag', + field: 'slug', + terms: ['featured'] + } + ], order: 'asc', orderBy: 'date' }, 📝 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. query: { perPage: 8, postType: 'accommodation', taxQuery: [ { taxonomy: 'post_tag', field: 'slug', terms: ['featured'] } ], order: 'asc', orderBy: 'date' }, align: 'wide' 555-556: 🧹 Nitpick (assertive) Ensure consistency in 'name' metadata for 'Related Tours - Accommodation' The name in metadata is 'Related Tour - Accommodation', whereas the title is 'Related Tours - Accommodation'. For consistency, please update the name to match the title. name: 'Related Tour - Accommodation' +name: 'Related Tours - Accommodation' Committable suggestion skipped: line range outside the PR's diff. 915-919:⚠️ Potential issue Add query parameters to fetch related reviews for destinations The 'Related Reviews - Destination' block should display reviews linked to the current destination. The query lacks filters to retrieve related reviews. Please update the query accordingly. query: { perPage: 8, postType: 'review', + metaQuery: [ + { + key: 'related_destination', + value: wp.data.select('core/editor').getCurrentPostId(), + compare: '=' + } + ], order: 'asc', orderBy: 'date' }, 📝 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. perPage: 8, postType: 'review', metaQuery: [ { key: 'related_destination', value: wp.data.select('core/editor').getCurrentPostId(), compare: '=' } ], order: 'asc', orderBy: 'date' }, 514-519:⚠️ Potential issue Add query parameters to fetch related tours The 'Related Tours - Tour' block is intended to display tours related to the current tour via shared destinations. The current query does not filter for related tours. Please include a taxQuery to fetch tours that share destinations with the current tour. query: { perPage: 8, postType: 'tour', + taxQuery: [ + { + taxonomy: 'destination', + field: 'term_id', + terms: wp.data.select('core/editor').getEditedPostAttribute('terms').destination || [], + operator: 'IN' + } + ], order: 'asc', orderBy: 'date' }, 📝 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. query: { perPage: 8, postType: 'tour', taxQuery: [ { taxonomy: 'destination', field: 'term_id', terms: wp.data.select('core/editor').getEditedPostAttribute('terms').destination || [], operator: 'IN' } ], order: 'asc', orderBy: 'date' }, 433-441:⚠️ Potential issue Add query parameters to fetch related accommodation The 'Related Accommodation - Tour' block aims to display accommodations related to the current tour via shared destinations. However, the query lacks filters to retrieve these related posts. Please update the query to include appropriate taxonomy queries. query: { perPage: 8, postType: 'accommodation', + taxQuery: [ + { + taxonomy: 'destination', + field: 'term_id', + terms: wp.data.select('core/editor').getEditedPostAttribute('terms').destination || [], + operator: 'IN' + } + ], order: 'asc', orderBy: 'date' }, 📝 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. metadata: { name: 'Related Accommodation Query' }, query: { perPage: 8, postType: 'accommodation', taxQuery: [ { taxonomy: 'destination', field: 'term_id', terms: wp.data.select('core/editor').getEditedPostAttribute('terms').destination || [], operator: 'IN' } ], order: 'asc', orderBy: 'date' }, assets/js/blocks/destination.js (4) 452-452:⚠️ Potential issue Use 'content' instead of 'text' in the 'core/button' block In the 'Banking' block variation, the core/button block uses the text attribute (line 452~) for the button label. This is inconsistent with other block variations where the content attribute is used. The text attribute is not recognised for core/button blocks and may lead to the button label not displaying correctly. Apply this diff to correct the attribute: }, - text: 'View More' + content: 'View More' 📝 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. content: 'View More' 1305-1341: 🛠️ Refactor suggestion Simplify 'core/button' structure in 'Safety' block variation In the 'Safety' block variation, the core/button block (lines 1305-1341) unnecessarily wraps content inside a nested core/paragraph block. This is inconsistent with other block variations and may cause rendering issues. The core/button block can directly use the content attribute. Refactor the core/button block to simplify its structure: 'core/buttons', {}, [ [ 'core/button', { backgroundColor: 'primary', width: 100, style: { border: { radius: { bottomLeft: '8px', bottomRight: '8px', } } }, - }, - [ - [ - 'core/paragraph', { - content: 'View More', - className: 'has-primary-background-color has-background wp-element-button', - style: { - border: { - bottomLeftRadius: '8px', - bottomRightRadius: '8px', - } - } - } - ] - ] + content: 'View More', + className: 'has-primary-background-color has-background wp-element-button' + } ] ] 📝 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. [ 'core/buttons', {}, [ [ 'core/button', { backgroundColor: 'primary', width: 100, style: { border: { radius: { bottomLeft: '8px', bottomRight: '8px', }, }, }, content: 'View More', className: 'has-primary-background-color has-background wp-element-button' } ], ], ], ], 1462-1496: 🛠️ Refactor suggestion Simplify 'core/button' structure in 'Visa' block variation Similarly, the 'Visa' block variation's core/button block (lines 1462-1496) uses a nested core/paragraph block. This complexity is unnecessary and inconsistent with standard practices. Using the content attribute directly simplifies the block and ensures consistency. Refactor the core/button block as follows: 'core/buttons', {}, [ [ 'core/button', { backgroundColor: 'primary', width: 100, style: { border: { radius: { bottomLeft: '8px', bottomRight: '8px', } } }, - }, - [ - [ - 'core/paragraph', { - content: 'View More', - className: 'has-primary-background-color has-background wp-element-button', - style: { - border: { - bottomLeftRadius: '8px', - bottomRightRadius: '8px', - } - } - } - ] - ] + content: 'View More', + className: 'has-primary-background-color has-background wp-element-button' + } ] ] 📝 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. 'core/buttons', {}, [ [ 'core/button', { backgroundColor: 'primary', width: 100, style: { border: { radius: { bottomLeft: '8px', bottomRight: '8px', }, }, }, content: 'View More', className: 'has-primary-background-color has-background wp-element-button' } ], ], ], 902-925: 🧹 Nitpick (assertive) Missing 'title' attribute in 'Dress' block variation The 'Dress' block variation (lines 902-925) lacks a title attribute under attributes. Including the title attribute helps maintain consistency and improves the block's identification in the editor. Consider adding the title attribute: attributes: { + title: 'Dress', metadata: { name: 'Dress', }, className: 'lsx-dress-wrapper', ... }, 📝 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. attributes: { title: 'Dress', metadata: { name: 'Dress', }, className: 'lsx-dress-wrapper', style: { border: { radius: '8px' }, spacing: { padding: { top: '0px', bottom: '0px', left: '0px', right: '0px' }, blockGap: '0px' } }, backgroundColor: 'base', layout: { type: 'constrained' } },
Description
Tour Operator has been re written to use the WordPress site editor. The beta release includes updates to the custom fields, the registering of the post types using the Create Content Model, as well as removal of heavy query based templates.
Pull Requests
Summary by CodeRabbit
Release Notes
New Features
Bindingsclass to manage custom fields for itineraries and accommodation units.Templatesclass for registering block templates, improving organization and usability.Bug Fixes
Refactor
Admin,Tour_Operator, andAccommodationclasses to simplify functionality and improve maintainability.Documentation
2.0.0, including the removal of the UIX vendor library and enhancements to settings management.