anseki / plain-draggable

The simple and high performance library to allow HTML/SVG element to be dragged.
https://anseki.github.io/plain-draggable/
MIT License
773 stars 99 forks source link

Angular, SVG, component with multiple handles #112

Closed michaelkariv closed 2 years ago

michaelkariv commented 2 years ago

I have made a test with Angular, SVG and draggable handle. I have made a separate component out of SVG, and it all works nicely. I summed up all the issues other people discussed regarding Angular, SVG etc in the readme.md

However here is the problem I am not sure how to solve. I want multiple draggable handles in a component. So I use *ngFor and ng-container to generate the code

<ng-container     *ngFor="let t of texts; let i=index"  
       <g  #draggables [attr.transform]="'translate(200,' + 400*i + ')'">     <circle...

Accordingly, in the ts @ViewChildren is used

  @ViewChildren('draggables')
  public handles: QueryList<ElementRef> | undefined;
...
        for (let i = 0; i < handles.length; i++) {
          const draggable = new PlainDraggable(handles[i].nativeElement);
        }

This works ok, until a bound property changes and the component gets redrawn. Dragging stops.

Full project illustrating the issue is in the public github project. https://github.com/michaelkariv/plain-draggable-angular-svg-test

To make dragging stop, just change the text of the label that appears beneath all the components.

anseki commented 2 years ago

Hi @michaelkariv, thank you for the comment. Sorry, my English is poor. Do you mean that the PlainDraggable instance that was passed <g> element didn't work?

michaelkariv commented 2 years ago

Hi @anseki , your English is great, don't worry about it. It is foreign to me too, but I perfectly understand what you say, and seems like you understand me too, and that is what it is for.

Regarding the problem. First, it is not necessarily a bug in your code. Probably it is Angular problem or mine. So what I need is guidance from you.

The back story is this. I am not even sure what exacly happens, internally. I am working on my own widget, a clock with multiple time markers, and things were great, until I connected it to the rest of the app. So I went back to my test application and modified it to represent the problem. If you look at the project in the repository (https://github.com/michaelkariv/plain-draggable-angular-svg-test), it is easily reproducible.

Here is what I tried in my own app and it is represented in the test repo. Seems that the event handlers to mouseDown and dragging(or similar, don't remember the exact name) disappeared from the DOM element of . I then created a function that reacts to changes and calls initialization again, So new PlainDraggable(handles[i].nativeElement) is called again.I did not dive into your code to see what PlainDraggable constructor is doing. I did one tiny test assuming that maybe class "plain-draggable" is an indication not to modify the element attaching new event handler, so I deleted that class before calling init again.

That did not help. So instead of all that guess work, I put effort into creating a minimal reproducible code and let you know.

I think you guidance for Angular/SVG is important not just to me. First for those who work in Angular, binding of data from external component is a basic thing, so this use case is standard. Second,if I am to guess, those who use Angular have all the incentives to use Angular Material, which now has it own drag class in the CDK, which works great for HTML DOM components, especially reordering lists. According to this stackoverflow item CDK drag used to work for Angular 7 but not for Angular 11.

Your library still offers some goodies even for HTML DOM, but those who do not need snapping might prefer CDK, I would guess. It cuts dependencies. But as I mentioned the CDK comes short is SVG. So what are the options? For SVG it is either your library or manually implementing dragging, or using heavy 3rd party libraries like d3 or green sock. I did not find any other solution. I would love to stay with your library if the solution to this problem can be found. I also promise to pack up my test to make life of others who develop Angular SVG components easier.

anseki commented 2 years ago

I see. Regardless of the library, you have to learn about Angular lifecycle. Try this: (drag-us.component.ts)

import {Component, ElementRef, Input, OnInit, QueryList, ViewChild, ViewChildren} from '@angular/core';

declare const PlainDraggable: any;

@Component({
  selector: 'app-drag-us',
  templateUrl: './drag-us.component.html',
})
export class DragUsComponent implements OnInit {

  constructor() {
  }

  @ViewChild('container', {static: true})
  public container: any;

  @ViewChildren('draggables')
  public handles: QueryList<ElementRef> | undefined;

  // @Input() texts: string[] = ['1', '2'];
  private _texts: string[] = ['1', '2'];

  public ngOnInit(): void {
  }

  @Input() set texts(value: string[]) {
    this._texts = value;
    setTimeout(() => { this.initDraggable(); }, 0);
  }

  get texts(): string[] { return this._texts; }

  public initDraggable(): void {
    const container = this.container.nativeElement;

    const options = {
      containment: container
    };
    if(!this.handles) return;
    const handles = this.handles.toArray();

    function init() {
      try {
        console.log(`initDraggable().init`);
        for (let i = 0; i < handles.length; i++) {
          const draggable = new PlainDraggable(handles[i].nativeElement);
        }
      } catch (error) {
        setTimeout(init, 200);
      }
    }

    init();
  }

  // ngAfterViewInit() {
  //   console.log('ngAfterViewInit()');
  //   this.initDraggable();
  // }

}
michaelkariv commented 2 years ago

I did try something like that, and it did not work, but just give me a minute, I'll check out your code UPDATE, your code works. I need to understand what I did differently. Current suspect is that I called initDraggable() syncronously, without setTimeout UPDATE2, your suggestions work in my own code. setTimeout did the trick.

anseki commented 2 years ago

I'm glad if I could help you. :smile: The setTimeout did not the trick, it is setter/getter. Note that that code is not good logic for performance. Typically, app should change only updated element.

michaelkariv commented 2 years ago

Sorry I bag to differ. I just tested it. setter and getter is not enough. I left a comment in my code. I am also writing a better README.md

michaelkariv commented 2 years ago

Have just updated the README.md and pushed all the code changes.

anseki commented 2 years ago

I'm glad if I could help you. :smile:

michaelkariv commented 2 years ago

Thank you so much. Gread library, perfect support of us humble users

michaelkariv commented 2 years ago

Oh, seems the initialization multiple times creates multiple instances of handlers for drag event, and then drag end event is fired multiple times . I will try and reproduce it in the test, but superficially, the old instance of PlanDraggable needs somehow be removed from the handle

michaelkariv commented 2 years ago

It also adds so many translate(0,0) to the handle, look <g _ngcontent-itp-c43="" transform="translate(113.63 396.472) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0) translate(0 0)" style="-webkit-tap-highlight-color: transparent; box-shadow: transparent 0px 0px 1px; cursor: pointer; user-select: none;" class="plain-draggable"><circle _ngcontent-itp-c43="" cx="0" cy="0" r="80" fill="rgba(255,252,216,0.7)" stroke="rgb(253,160,26)" stroke-width="10"></circle><text _ngcontent-itp-c43="" x="0" y="0" dominant-baseline="middle" text-anchor="middle" fill="rgb(253,160,26)" font-size="48pt" font-family="'Impact', sans-serif">1 </text></g>

michaelkariv commented 2 years ago

In summary, I think some cleanup is due before creating yet another PlainDraggable wrapper.

anseki commented 2 years ago

Of course you should not use that code for standard app. As I said, that code has problems. https://github.com/anseki/plain-draggable/issues/112#issuecomment-1179677461 At least, you should manage current instance of libraries (PlainDraggable, and others). For example:

let draggable1, draggable2;
  :
// Initialize
if (draggable1) {
  draggable1.remove();
  draggable1 = null;
}
if (draggable2) {
  draggable2.remove();
  draggable2 = null;
}
draggable1 = new PlainDraggable(...);
draggable2 = new PlainDraggable(...);

This is typical case. You may improve it more.

michaelkariv commented 2 years ago

Yes, I tried remove() , found it in your soruce code, but for some reason it does not help. I'll play with it more and report back