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/control draggability of individual points #158

Closed

Conversation

rendertom
Copy link
Contributor

@rendertom rendertom commented Jan 2, 2023

User story:
As a user I want to have control over the draggability of particular set of selected points without modifying the initial selection.

Description:
This PR exposes additional API for the developer to change the draggability option for a set of particular points.

  • canBeDragged(element): checks whether the element can be dragged.
  • makeDraggable(element): make element draggable.
  • makeNonDraggable(element): make element non-draggable.

Usage example:

const ds = new DragSelect({
  selectables: document.getElementsByClassName('item'),
  area: document.getElementById('container'),
  draggability: true, // optional, but has to be set 'true' to control draggability of individual points
  noDragClass: 'no-drag', // optional, defaults to 'ds-no-drag'
});

// Make every other element non-draggable
ds.subscribe('predragstart', (data) => {
  if (data.isDragging) {
    data.items.forEach((item, index) => {
      index % 2 === 0 // index is even
        ? ds.makeDraggable(item)
        : ds.makeNonDraggable(item);
    });
  }
});

Copy link
Owner

@ThibaultJanBeyer ThibaultJanBeyer left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @rendertom! This makes sense! 🎉

I left a few comments and would ask you to also do following:

  • Add a test for this (check other tests to see how it's done, it's very simple)
  • Add the information to the online docs in the www folder
  • Merge latest master

Then I'll merge! Thank you!!

@@ -377,6 +377,27 @@ class DragSelect {
*/
getItemsInsideByZoneId = (zoneId, addClasses) =>
this.DropZones.getItemsInsideById(zoneId, addClasses)

/**
* Make element draggable
Copy link
Owner

Choose a reason for hiding this comment

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

I guess the description is wrong

* @param {DSElement} element
* @returns {Boolean} whether the element can be dragged
*/
canBeDragged = (element) => this.SelectableSet.canBeDragged(element)
Copy link
Owner

Choose a reason for hiding this comment

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

maybe rename to isDraggable. Which is clearer that it's a question than a setter.

@@ -35,6 +35,7 @@
* @property {string} [dropZoneReadyClass=ds-dropzone-ready] on corresponding dropZone when element is dragged
* @property {string} [dropZoneTargetClass=ds-dropzone-target] on dropZone that has elements from any successful target drop
* @property {string} [dropZoneInsideClass=ds-dropzone-inside] on dropZone that has elements inside after any drop
* @property {string} [noDragClass=ds-no-drag] the class assigned to a non-draggable element
Copy link
Owner

Choose a reason for hiding this comment

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

Hmm can you clarify this? As of now then if I turn draggability off in the settings, all elements should have this class, right? Maybe think about a more precise description 🤔

@ThibaultJanBeyer
Copy link
Owner

ThibaultJanBeyer commented Sep 25, 2023

Hey there, I’m visiting this as part of v3. I’ll only take over the class, then, as a developer, I can just add/remove the class to whatever element whenever I want which in turn stops the dragging on an individual element, I think that’s quite cool! Closing for now, expect to see it in v3

Edit: or actually what do you think about a custom filter override? Like we have for filterSelected (docs)? I.e. a filterDragElements override… I tend to prefer that approach because, sure it’s slightly more complex, but it gives users the full power of control 🤔

@ThibaultJanBeyer ThibaultJanBeyer mentioned this pull request Sep 25, 2023
Merged
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.

None yet

2 participants