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.

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

Feature: Slider - Draggable functionality #93

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

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