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

EZP-29606: As a developer I want to have a generic interface for drag and drop interactions #617

Merged
merged 1 commit into from Sep 13, 2018

Conversation

sunpietro
Copy link
Contributor

@sunpietro sunpietro commented Sep 12, 2018

Question Answer
Tickets https://jira.ez.no/browse/EZP-29606
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? yes
License GPL-2.0

FOR QA

There is nothing to test in this PR. This developer feature will be used in other PRs.

How to use it?

Prepare the HTML structure like this:

<div class="items-container" data-placeholder="HTML template for placeholder"></div>

In your JS code follow the convention:

const draggable = new global.eZ.core.Draggable({
        itemsContainer: HTMLElement of .items-container,
        selectorItem: String,
        selectorPlaceholder: String,
});

Options

  • itemsContainer - a reference to DOM node containing a draggable items,
  • selectorItem - a CSS selector of a draggable item,
  • selectorPlaceholder - a CSS selector of a placeholder,
  • afterInit - a callback function invoked after interface initialization (optional),
  • afterDragStart - a callback function invoked after starting to drag (optional),
  • afterDragOver - a callback function invoked after moving onto a droppable element (optional),
  • afterDrop - a callback function invoked after dropping an element (optional),
  • attachCustomEventHandlersToItem - a function to be invoked while attaching event handlers to every item in the items container (optional).

const onDragOverTimeout = Symbol('onDragOverTimeout');

class Draggable {
constructor(options) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we call it everywhere config, not options, so maybe it would be a good idea to call it config here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, let's change it to config

attachCustomEventHandlersToItem: () => {},
};

const attachEventHandlersToItem = Symbol('attachEventHandlersToItem');
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a new way we are going to protect private methods?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, exactly.

const TIMEOUT_REMOVE_PLACEHOLDERS = 500;
const DEFAULTS = {
afterInit: () => {},
afterDragStart: () => {},
Copy link
Contributor

Choose a reason for hiding this comment

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

For me sticking with original convention and naming it onDragStart would be better. onDragStart is more precise (after may be even after DragEnd) and is well known.
I know that it might be confusing from class perspective but we may change class methods's names to e.g. handleDragStart or change something else etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, but these methods are always fired after everything is done in our dragstart or dragover handlers. That's why I named these callbacks as afterDragStart and afterDragOver

@sunpietro sunpietro force-pushed the ezp-29606-drag-drop-core-interface branch from 7cb5144 to 86875e7 Compare September 13, 2018 09:06
@sunpietro sunpietro merged commit 0658cce into master Sep 13, 2018
@sunpietro sunpietro deleted the ezp-29606-drag-drop-core-interface branch September 13, 2018 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants