Skip to content

Conversation

@AvinashVinod03
Copy link

This PR introduces draggable functionality to the TP Slider component. It enables users to drag the slider using mouse events (mousedown, mousemove, mouseup) for improved navigation. The PR also includes necessary updates to the slider's event listeners and ensures smooth dragging behavior.

Copy link

@nrvarun nrvarun left a comment

Choose a reason for hiding this comment

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

@AvinashVinod03 Added some notes.

handleMouseOver() {
// Only styles when the draggable will be yes.
if ( this.getAttribute( 'draggable' ) === 'yes' ) {
this.classList.add( 'tp-slider--grab' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.

handleMouseOut() {
// Reset cursor if not dragging.
if ( ! this.isDragging ) {
this.classList.remove( 'tp-slider--grab' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.

this.dragOffsetX = 0;

// Reset cursor and user-select styles
this.classList.remove( 'tp-slider--grabbing' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.


// Reset cursor and user-select styles
this.classList.remove( 'tp-slider--grabbing' );
this.classList.add( 'tp-slider--grab' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.

// Reset cursor and user-select styles
this.classList.remove( 'tp-slider--grabbing' );
this.classList.add( 'tp-slider--grab' );
this.classList.remove( 'tp-slider--dragging' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.

event.preventDefault();

// Apply the grabbing cursor class
this.classList.add( 'tp-slider--grabbing' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.


// Apply the grabbing cursor class
this.classList.add( 'tp-slider--grabbing' );
this.classList.remove( 'tp-slider--grab' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.

this.classList.remove( 'tp-slider--grab' );

// Prevent text selection
this.classList.add( 'tp-slider--dragging' );
Copy link

Choose a reason for hiding this comment

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

@AvinashVinod03 lets use javascript to do the style updates.

@@ -0,0 +1,62 @@
tp-slider {
Copy link
Contributor

Choose a reason for hiding this comment

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

@AvinashVinod03 Is this committed by mistake?

Copy link
Author

Choose a reason for hiding this comment

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

No @junaidbhura, as this is for the draggable functionality example.

/**
* Check and initialize draggable behavior.
*/
checkDraggable() {
Copy link
Contributor

Choose a reason for hiding this comment

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

@AvinashVinod03 I wonder if draggable should be implied by swipe rather than creating a separate attribute? Have we evaluated the pros and cons?

event.preventDefault();

// Update styles for grabbing and disable text selection.
this.style.cursor = 'grabbing';
Copy link
Contributor

Choose a reason for hiding this comment

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

@AvinashVinod03 Perhaps this should be attributes on the component itself. Please avoid using CSS in JS

Copy link
Author

Choose a reason for hiding this comment

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

@junaidbhura do you want that we want to use classList.add and give styles with the css file itself ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants