katoid / angular-grid-layout

Responsive grid with draggable and resizable items for Angular applications.
https://katoid.github.io/angular-grid-layout
MIT License
462 stars 61 forks source link

Drag Handle Inside Child Component #86

Closed pcurrivan closed 1 year ago

pcurrivan commented 1 year ago

Putting the ktdGridDragHandle directive inside a child component does not work:

<ktd-grid-item><my-item></my-item></ktd-grid-item>

my-item.component.ts: <div ktdGridDragHandle ></div> <div>My Item Content</div>

I modified the stackblitz Custom Handles example to show this: https://stackblitz.com/edit/angular-grid-layout-playground-dfe5up?file=src/app/custom-handles/my-item/my-item.component.html

I believe this is related to this issue *

Is there a workaround for this that allows me to use a component inside the ktd-grid-item?

I tried making a HostListener in my-item and calling stopPropagation / stopImmediatePropagation on all mousedown events except for those from the drag handler element, but this breaks the intended mousedown functionality of the item content.

Should this perhaps be event-based? (my-item could Output mousedown and mouseup from its internal handle element, and ktd-grid-item could handle the events)

* EDIT: This is not due to an issue with Angular - it is working as intended for ContentChildren to be unable to find elements in child components: https://angular.io/api/core/ContentChildren#description "Does not retrieve elements or directives that are in other components' templates, since a component's template is always a black box to its ancestors."

llorenspujol commented 1 year ago

Thanks for explaining this well! 👏

I understand the problem, and I agree we should find a solution to make it work.

The solution your proposed works for me. We can let users start the drag sequence manually. This can also solve other use cases, so it will be a positive addition.

The only thing I am not totally convinced about is using the Input enableManualDragEvents. What about solving it by just using the function onManualDragStart(event)? (I also would suggest rename this function to something like startDragManually(event) or startDrag(event))

And if you want to disable the default dragging, you can always set the grid item to be [draggable]="false"

The usage may look like this 👇

Angular Grid Layout template:

<ktd-grid [cols]="cols"
          [rowHeight]="rowHeight"
          [layout]="layout"
          (layoutUpdated)="onLayoutUpdated($event)">
    <ktd-grid-item *ngFor="let item of layout; trackBy:trackById" [id]="item.id" [draggable]="false">
        <my-grid-item>
        </my-grid-item>
    </ktd-grid-item>
</ktd-grid>

MyGridItemComponent:

@Component({
    selector: 'my-grid-item'
})
export class MyGridItemComponent {

    constructor(@Optional() gridItem: KtdGridItemComponent) { }

    onDragStart(event) {
        gridItem.startDragManually(event);
    }
}

What are your thoughts about it?

pcurrivan commented 1 year ago

I originally did not have the enableManualDragEvents flag, but then I realized I still wanted to be able to toggle dragging on and off in the grid while using manual dragging, which doesn't work if you need to set draggable to false to use manual dragging. So the flag seemed like the most straight forward way to switch between dragging modes while still allowing switching dragging on and off. It also seemed confusing to set draggable to false but still have dragging enabled in a way.

I can of course change the method name.

llorenspujol commented 1 year ago

@pcurrivan It would be really nice to avoid another input. I understand that it makes sense for your use case, but in my opinion, it seems like too much for the overall Angular Grid Layout API.

If I understood correctly, you want to toggle dragging on and off while using manual dragging. Currently, it may not work, but I think the key is to change the code to allow it. In my opinion, manual dragging should work independently of the value of draggable. So, even if draggable is false, you should still be able to use manual dragging.

I drafted a possible solution:

        return merge(
            this._manualDragEvents$,
            this._draggable$.pipe(
                switchMap((draggable) => {
                        if (!draggable) {
                            return NEVER;
                        } else {
                            return this._dragHandles.changes.pipe(
                                startWith(this._dragHandles),
                                switchMap((dragHandles: QueryList<KtdGridDragHandle>) => {
                                    return iif(
                                        () => dragHandles.length > 0,
                                        merge(...dragHandles.toArray().map(dragHandle => ktdMouseOrTouchDown(dragHandle.element.nativeElement, 1))),
                                        ktdMouseOrTouchDown(this.elementRef.nativeElement, 1)
                                    ).pipe(
                                        exhaustMap((startEvent) => {
                                            return this._applyDragThreshold$(startEvent);
                                        })
                                    );
                                })
                            );
                        }
                    }
                ))
        ).pipe(
            exhaustMap(e => this._applyDragThreshold$(e))
        );

Let me know your thoughts about it!

pcurrivan commented 1 year ago

@llorenspujol I removed the Input as requested and updated the comments.