Feature/drag drop v2 #194

Mboulianne · 2018-04-13T18:48:28Z

`@ulaval/modul-components`

PR Checklist

Provide a small description of the changes introduced by this PR
V2 du drag and drop.
Include links to issues
MODUL-21

Include this section in the release notes
Drag and drop v2
Ajout d'une classe m--is-grabbing quand on "grab" un élément draggable.
Gestion de l'update dynamiques des propriétés des directives. (:x-y-z="abc")
Permettre le scroll sur les entête position: fixed. v-m-draggable-allow-scroll. Fix s'applique seulement si le polyfill est actif.
Ne pas permettre la sélection (long touch, double click, etc) sur les éléments draggable ou droppable.
Gestion du ghost image (sauf sur edge)
Ajout de quelques tests pour les bouts les plus simples. (à suivre)
Assurer que les transition-group fonctionnent
Beaucoup de fix pour bug de compatibilité cross-browser
Other info
Début d'une doc pour confluence pour aider les devs à débuter l'intégration.
Doc Sortable/Droppable/Sortable

-Fix some flickers issues in sortable (again!) -Remove the possibility to select text in the draggable / droppable container.

Fix insert parameter in droppable directive bind

Rework droppable binding. Rework sortable binding. Fix undefined accepted actions on sortable

First work on sortable insertion behavior

Mboulianne · 2018-04-13T18:48:56Z

Je submit la PR.. mais je dois encore terminer 1 truc ou 2 lundi.

Je submit tout suite qu'on puisse la regarder pour éventuellement release mercredi

add event.stopPropagation where events are handled by custom code. Add m--is-grabbing class for desktop

Support is limited.. will fully work on edge but will only partially work fir all other browsers

…rify if the element has a vue instance. If it does, emit the event through the $emit service. Else, use native element.dispatchEvent

fix droppable leave on firefox.

… everything is detached when the binding of each direction = false

…he element after a drop if the drop wasn't accepted.

Mboulianne · 2018-04-20T13:03:26Z

src/directives/domPlugin.ts

+            element[constructorFunction.defaultMountPoint] = plugin;
+            return plugin;
+        } else {
+            return new NullObjectMElementPlugin(options);


Si le plugin est finalement pas mount pour une raison quelconque, je retourne un null object pour avoir à fairep lein de if (pluginExist) partout dans le code. C'tait trop tannant

Mboulianne · 2018-04-20T13:04:26Z

src/directives/draggable/draggable.ts

+        if (this.options.canDrag === undefined) { this.options.canDrag = true; }
+        if (this.options.canDrag) {
+            mount(() => {
+                this.element.classList.add(MDraggableClassNames.Draggable);


le mount permet au attach de décider, selon une condition, si on veut ou pas associer la logique de directive à l'élément.

Mboulianne · 2018-04-20T13:07:00Z

src/directives/domPlugin.ts

+    Mounted = 'mounted',
+    UnMounted = 'unmounted'
+}
+export abstract class MElementPlugin<OptionsType> implements IMElementPlugin<OptionsType> {


J'ai jamais expliqué pk j'ai eu à faire ça mais voilà =>
Avec le drag and drop je dois être capable de gérer les 2 situations suivantes =>
1- pouvoir drag / drop / sort en associant une directive à l'élément

2- je dois aussi être en mesure de "composer" dynamiquement le comportement d'une directive. Pour éviter la duplication de code, fallait donc que j'extract la logique des directives sortable / droppable afin de pouvoir les composer dans sortable :)

Mboulianne · 2018-04-20T13:09:05Z

src/directives/sortable/sortable.ts

+    getInsertPosition(event: MDropEvent): MSortInsertPositions;
+}
+
+export class MSortableDefaultInsertionMarkerBehavior implements MSortableInsertionMarkerBehavior {


Permet de définir le comportement de sort before / after. On pourrait ensuite imaginer qu'un component implémente un interface semblable et laisser le component dicter le comportement du "insert" (before, on, after)

Mboulianne · 2018-04-20T13:11:43Z

src/directives/sortable/sortable.ts

-            this.showPlaceHolder();
-            element.insertAdjacentElement(insertPosition, this.placeHolderElement);
+    private getInsertionMarkerBehavior(): MSortableInsertionMarkerBehavior {
+        /*const droppable: MDroppable = MDroppable.currentHoverDroppable as MDroppable;


Sera commenté out dasns une prochaine itération

Mboulianne · 2018-04-20T13:12:49Z

src/directives/user-select/remove-user-select.ts

+import { DirectiveOptions, VNodeDirective, VNode, PluginObject } from 'vue';
+import { REMOVE_USER_SELECT } from '../directive-names';
+
+export class MRemoveUserSelect extends MElementPlugin<boolean> {


Directive qui enlève la possibilité au user de
Sélectionner du texte en "draggant" dessus sur desktop.
En double clickant sur desktop.
En faisant un long touch sur mobile

Mboulianne · 2018-04-20T13:14:19Z

src/utils/vue/directive.ts

+    __vue__: any;
+}
+export const dispatchEvent = (element: HTMLElement, eventName: string, eventData: any): void => {
+    const vueElement: VueElement = element as VueElement;


Ceci est un peu un hack. Mes directives Droppable / Sortables / Draggable doivent pouvoir émettre des events dans le bus de Vue JS si possible. En gros, si on mount la directive sur un component, il est impératif qu'on $emit via l'instance vue.. sinon l'évènement se perd dans le néant.

Mboulianne · 2018-04-24T14:12:08Z

src/directives/draggable/draggable-allow-scroll.ts

@@ -0,0 +1,89 @@
+import { DRAGGABLE_ALLOW_SCROLL } from '../directive-names';


Permet le scroll natif des browsers sur desktop quand on dragOver sur un élément position: fixed. C'est un peu hacky mais ça règle notre problème pour pas cher.

simardo

Encore une fois j'ai noté quelques petites choses mineures au niveau syntaze principalement. Honnêtement il y a beaucoup de trop de code pour que je puisse regarder le "mécanique" de tout ça. Éventuellement faudrait prévoir un peu de temps pour que tu présentes tout ça à quelques développeurs, ne serait-ce que pour en faire la maintenance.
Tu rajouteras l'info nécessaire dans la description du PR #194 et on merge!

simardo · 2018-04-24T20:08:12Z

src/directives/directive-names.ts

@@ -1,7 +1,9 @@
 export const DRAGGABLE: string = 'm-draggable';
+export const DRAGGABLE_ALLOW_SCROLL: string = 'm-draggable-allow-scroll';


Mets un suffixe _NAME à tes noms de directives. ça m'avait échappé le premier coup puisque je voyais juste les nouvelles directives dans le file-change-viewer de github.

Done. J'ai fix TEXTAREA_AUTO_HEIGHT au passage

simardo · 2018-04-24T20:13:37Z

src/directives/domPlugin.ts

+    }
+}
+
+export interface IMElementPlugin<OptionsType> {


Pas besoin de préfixer le nom de l'interface avec un "I". Les internets ont déjà explosé à ce sujet :) microsoft/TypeScript-Handbook#121

Pas de troubles avec ça!

simardo · 2018-04-24T20:19:57Z

src/directives/domPlugin.ts

    protected attachedEvents: Map<string, EventListenerOrEventListenerObject[]> = new Map<string, EventListenerOrEventListenerObject[]>();
    protected _options: OptionsType;
    private readonly _element: HTMLElement;
-    public get element(): HTMLElement { return this._element; }
+    private _status: MElementPluginStatus = MElementPluginStatus.UnMounted;


Côté convention, je ne m'y étais pas vraiment attardé, mais je ne crois pas utile de préfixer par un ' _ ' les attributs private. Typiquement, si tu déclares l'attribut privé, le "compilateur" ne te laissera pas y accéder à l'extérieur de la classe. J'ai vu par contre la convention du ' _ ' dans du pur Javascript, puisque tout est accessible de partout. Typescript lui-même va générer des attributs préfixés de ' _ ' lorsqu'il produit le Javascript.

J'ai enlevé le status.

Dans les faits j'utilise _someThing quand on cache une propriété derrière un get / set

private myProperty;
public get myProperty() { return this..myProperty }

Bon point pour le cas du get/set

simardo · 2018-04-24T20:24:24Z

src/directives/draggable/draggable.ts

-    bind(element: HTMLElement, binding: VNodeDirective, node: VNode): void {
-        MDOMPlugin.attach(MDraggable, element, extractVnodeAttributes(node));
+    inserted(element: HTMLElement, binding: VNodeDirective, node: VNode): void {
+        const test = MDOMPlugin.attach(MDraggable, element, extractVnodeAttributes(binding, node));


à quoi sert le "test" ici?

À rien. Oublie

simardo · 2018-04-25T12:44:38Z

src/directives/draggable/draggable.ts

-import { MDroppable } from '../droppable/droppable';
+import { MElementPlugin, MDOMPlugin, MountFunction, RefreshFunction } from '../domPlugin';
+import { getVNodeAttributeValue, dispatchEvent } from '../../utils/vue/directive';
+import { MDroppable, MDroppableClassNames } from '../droppable/droppable';


MDroppableClassNames ne semble pas utilisé dans la classe

simardo · 2018-04-25T12:44:50Z

src/directives/draggable/draggable.ts

+import { MDroppable, MDroppableClassNames } from '../droppable/droppable';
+import { clearUserSelection } from '../../utils/selection/selection';
+import { MRemoveUserSelect } from '../user-select/remove-user-select';
+import { MSortable } from '../sortable/sortable';


MSortable ne semble pas utilisé dans la classe

@Mboulianne Installe l'extension typescript hero pour auto nettoyer tes imports au save

Je l'avais déjà install.. je l'avais juste pas config comme du monde :(

Mboulianne added 13 commits April 10, 2018 09:43

Drag ghost part 1

bed3b65

-Remove user selection when dragging

0af918b

-Fix some flickers issues in sortable (again!) -Remove the possibility to select text in the draggable / droppable container.

disable text selection on mobile

a910c49

Rename sortable group

eb80981

add back droppablep lugin in index.ts

705700b

Fix insert parameter in droppable directive bind

Rework draggable binding.

4fc1b5e

Rework droppable binding. Rework sortable binding. Fix undefined accepted actions on sortable

forgot to stage changes

0949d49

Remove element placeholder

7e574e7

cleanup logging

b3c29e3

Remove sortable grouping option since it's useless.

0e2a667

First work on sortable insertion behavior

documentation review

a1d951d

fix droppable bug that made iti mpossible to drop

9768485

Remove superfluous code

bfb46ec

Mboulianne added the enhancement label Apr 13, 2018

Mboulianne added 14 commits April 16, 2018 15:31

add m--is-droppable / draggable to droppable / draggable.

d90e73e

add event.stopPropagation where events are handled by custom code. Add m--is-grabbing class for desktop

Add support for m-grabbing class.

0c5de25

Support is limited.. will fully work on edge but will only partially work fir all other browsers

performance improvement: only apply classes when absolutely needed

18f92c4

when dispatching an event from Droppable, Sortable or Draggable => Ve…

24008d9

…rify if the element has a vue instance. If it does, emit the event through the $emit service. Else, use native element.dispatchEvent

fix dragImage

185ee18

fix droppable leave on firefox.

remove console.log, hide dragImage

9e24513

fix chrome clipping / artifact with drag image. (when dragging)

9b2940c

Allow to dyanmically decide if you can sort / drop / drag by ensuring…

2932a8f

… everything is detached when the binding of each direction = false

bugFix (edge/IE) fixed a bug where sortable classes would remain on t…

edbcf2d

…he element after a drop if the drop wasn't accepted.

fix the fix

a41f41e

Fix plugin refreshing + toggle

1af3140

when sortable is false at the beginning, hide dragImages anyway

a8a1190

resolve flickeri ssue

820d9b3

Code review

ce4f5c8

Mboulianne commented Apr 20, 2018

View reviewed changes

Mboulianne added 3 commits April 20, 2018 15:19

refactor plugin update

013d37b

fix scroll behavior when dragging on fixed elements

3d3753b

typo

e77afb7

Mboulianne commented Apr 24, 2018

View reviewed changes

Mboulianne added 2 commits April 24, 2018 15:24

test for remove-user-select

1fcae78

droppable-group tests

bae726f

simardo suggested changes Apr 25, 2018

View reviewed changes

Mboulianne and others added 6 commits April 25, 2018 09:23

rearrange import + remove useless ones

befac6d

code review

2dadcdc

Code review

f02801a

test begin for draggable

1ae8845

merge from branch 'develop'

e0dd75c

remove sortable-group

8b06ece

simardo approved these changes Apr 26, 2018

View reviewed changes

simardo merged commit d013a4b into develop Apr 26, 2018

simardo deleted the feature/drag-drop-v2 branch April 26, 2018 13:18

simardo added this to the 1.0.0-beta.30 milestone Apr 26, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/drag drop v2 #194

Feature/drag drop v2 #194

Mboulianne commented Apr 13, 2018 •

edited

Loading

Mboulianne commented Apr 13, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 20, 2018

Mboulianne Apr 24, 2018

simardo left a comment

simardo Apr 24, 2018

Mboulianne Apr 25, 2018

simardo Apr 24, 2018

Mboulianne Apr 25, 2018

simardo Apr 24, 2018

Mboulianne Apr 25, 2018

simardo Apr 26, 2018

simardo Apr 24, 2018

Mboulianne Apr 25, 2018

simardo Apr 25, 2018

simardo Apr 25, 2018

jfnadeau Apr 25, 2018

Mboulianne Apr 25, 2018 •

edited

Loading

		@@ -0,0 +1,89 @@
		import { DRAGGABLE_ALLOW_SCROLL } from '../directive-names';

		@@ -1,7 +1,9 @@
		export const DRAGGABLE: string = 'm-draggable';
		export const DRAGGABLE_ALLOW_SCROLL: string = 'm-draggable-allow-scroll';

Feature/drag drop v2 #194

Feature/drag drop v2 #194

Conversation

Mboulianne commented Apr 13, 2018 • edited Loading

@ulaval/modul-components

PR Checklist

Mboulianne commented Apr 13, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simardo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mboulianne Apr 25, 2018 • edited Loading

Choose a reason for hiding this comment

Mboulianne commented Apr 13, 2018 •

edited

Loading

`@ulaval/modul-components`

Mboulianne Apr 25, 2018 •

edited

Loading