Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Travel Information Slider #458

Merged
merged 6 commits into from
Dec 17, 2024
Merged

Travel Information Slider #458

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e018fe7
Travel Information Slider
Justinabes007 Dec 13, 2024
0406cff
Updating the slider js and CSS.
krugazul Dec 17, 2024
f273705
Updating the plugin header #369
krugazul Dec 17, 2024
8250fb9
Slider updates
krugazul Dec 17, 2024
3deaa0b
Fixing the travel info read more
krugazul Dec 17, 2024
dc325c5
Added in the travel info pattern
krugazul Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion assets/css/scss/_slider.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,19 @@
.wp-block-query {
.lsx-to-slider {
&.travel-information {
.slick-list {
max-width: 100% !important;
width: 100% !important;
}

.slick-slide {
padding: 0 !important;
margin: 0 10px !important;
}
}
Comment on lines +7 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

The use of !important is generally discouraged in CSS as it increases the specificity of a rule, making it harder to override if needed. It's better to increase specificity in a more controlled way, such as by using more specific selectors or by structuring your CSS so that the most specific rules are declared last.

- .slick-slide {
-   padding: 0 !important;
-   margin: 0 10px !important;
- }
+ .wp-block-query.lsx-to-slider.travel-information .slick-slide {
+   padding: 0;
+   margin: 0 10px;
+ }

Comment on lines +8 to +11
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

The use of !important is generally discouraged in CSS as it breaks the natural cascading in your stylesheets. It's better to increase specificity of the selector or rethink your class structure instead.

- .slick-slide {
-   padding: 0 !important;
-   margin: 0 10px !important;
- }
+ .lsx-to-slider.travel-information .slick-slide {
+   padding: 0;
+   margin: 0 10px;
+ }

}

.wp-block-query,
.lsx-travel-information-wrapper {
&.lsx-to-slider {
gap: var(--wp--preset--spacing--small) var(--wp--preset--spacing--small);
position: relative;
Expand Down
151 changes: 25 additions & 126 deletions assets/css/style.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion assets/css/style.css.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

73 changes: 60 additions & 13 deletions assets/js/src/custom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,43 +197,90 @@ if ( window.location.hash ) {
* @subpackage scripts
*/
lsx_to.build_slider = function( window_width ) {
// First slider: .lsx-to-slider
$( '.lsx-to-slider .wp-block-post-template:not(.slider-disabled)' ).each( function() {
var $this = $( this );

lsx_to.pre_build_slider( $this );

if ( 1 < $this.children.length ) {


if ( 1 < $this.children().length ) {
$this.slick( {
draggable: false,
infinite: true,
swipe: false,
dots: false,
slidesToShow: 3, // Show 3 items at a time
slidesToScroll: 1, // Scroll 1 item at a time
autoplay: false,
autoplaySpeed: 0,
appendArrows: $this.parent(), // Ensure arrows are appended correctly
appendDots: $this.parent(), // Append dots in the right container
responsive: [
{
breakpoint: 1028,
settings: {
slidesToShow: 2,
slidesToScroll: 1,
draggable: true,
arrows: true,
swipe: true,
dots: true,
}
},
{
breakpoint: 782,
settings: {
slidesToShow: 1,
slidesToScroll: 1,
draggable: true,
arrows: true,
swipe: true,
dots: true,
}
}
]
} );
Comment on lines +202 to +244
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

The changes made to the first slider look good. The addition of appendArrows and appendDots ensures that navigation arrows and dots are appended correctly, improving user interaction. The responsive settings have been updated to show different numbers of slides based on the screen size, which will enhance the user experience across various devices.

+ 					appendArrows: $this.parent(),  // Ensure arrows are appended correctly
+ 					appendDots: $this.parent(),    // Append dots in the right container
+ 					responsive: [
+ 						{
+ 							breakpoint: 1028,
+ 							settings: {
+ 								slidesToShow: 2,
+ 								slidesToScroll: 1,
+ 								draggable: true,
+ 								arrows: true,
+ 								swipe: true,
+ 								dots: true,
+ 							}
+ 						},
+ 						{
+ 							breakpoint: 782,
+ 							settings: {
+ 								slidesToShow: 1,
+ 								slidesToScroll: 1,
+ 								draggable: true,
+ 								arrows: true,
+ 								swipe: true,
+ 								dots: true,
+ 							}
+ 						}
+ 					]

}
} );

// Second slider: .lsx-to-slider.travel-information
$( '.lsx-travel-information-wrapper.lsx-to-slider .travel-information:not(.slider-disabled)' ).each( function() {
var $this = $( this );

lsx_to.pre_build_slider( $this );

// Ensure the second slider has 4 slides showing
if ( 1 < $this.children().length ) {
$this.slick( {
draggable: false,
infinite: true,
swipe: false,
dots: false,
slidesToShow: 3,
slidesToScroll: 1,
slidesToShow: 4, // Show 4 items at a time
slidesToScroll: 1, // Scroll 1 item at a time
autoplay: false,
autoplaySpeed: 0,
//appendArrows: $this.parent(),
//appendDots: $this.parent(),
appendArrows: $this.parent(), // Ensure arrows are appended correctly for this slider
appendDots: $this.parent(), // Append dots in the correct place
responsive: [
{
breakpoint: 1028,
settings: {
slidesToShow: 2,
slidesToShow: 3,
slidesToScroll: 1,
draggable: true,
arrows: false,
arrows: true,
swipe: true,
dots: true,
}
},
{
breakpoint: 782,
settings: {
slidesToShow: 1,
slidesToShow: 1,
slidesToScroll: 1,
draggable: true,
Comment on lines +248 to 284
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

The changes made to the second slider, .lsx-to-slider.travel-information, look good. The number of slides shown at different breakpoints has been updated according to the PR description. However, I noticed that the autoplaySpeed is set to 0 which means the autoplay speed is essentially non-existent. If you want to disable autoplay, you should set autoplay: false (which you've already done). But if you want autoplay to work, you should set autoplaySpeed to a positive integer representing the delay between auto slides in milliseconds.

-  autoplaySpeed: 0,
+  autoplaySpeed: 3000, // for example, change slides every 3 seconds

arrows: false,
arrows: true,
Comment on lines +244 to +285
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Image description CodeRabbit

The autoplaySpeed is set to 0, which means the slides will change instantly. This could lead to a poor user experience as users may not have enough time to read or interact with the content of each slide. Consider setting a reasonable value for autoplaySpeed.

- 					autoplaySpeed: 0,
+ 					autoplaySpeed: 3000, // 3 seconds

swipe: true,
dots: true,
}
Expand All @@ -243,7 +290,7 @@ if ( window.location.hash ) {
}
} );
};

/**
* Slider Lightbox.
*
Expand Down
9 changes: 9 additions & 0 deletions includes/classes/blocks/class-registration.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,12 +300,19 @@ public function maybe_hide_varitaion( $block_content, $parsed_block, $block_obj
}

$pattern = "/(lsx|facts)-(.*?)-wrapper/";
//$pattern = "/(lsx|facts)-((?:\w+-?)+)-wrapper/";
preg_match( $pattern, $parsed_block['attrs']['className'], $matches );

if ( empty( $matches ) ) {
return $block_content;
}

do_action( 'qm/debug', $matches );

if ( in_array( 'travel-information', $matches ) ) {
return $block_content;
}

if ( ! empty( $matches ) && isset( $matches[0] ) ) {
// Save the first match to a variable
$key = str_replace( [ 'facts-', 'lsx-', '-wrapper' ], '', $matches[0] );
Expand Down Expand Up @@ -385,6 +392,8 @@ public function maybe_hide_varitaion( $block_content, $parsed_block, $block_obj

foreach ( $key_array as $meta_key ) {
$value = lsx_to_custom_field_query( $meta_key, '', '', false );

do_action( 'qm/debug', $value );

// we need to see if the posts exist before we can use them
if ( stripos( $meta_key, '_to_' ) && 0 === $this->post_ids_exist( $value ) ) {
Expand Down
Loading