Opened 4 years ago

Closed 4 years ago

#12613 closed New Feature (fixed)

Show user that he can not drop on toolbar/ui on dragover

Reported by: Piotr Jasiun Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.5.0 Beta
Component: General Version:
Keywords: Cc:

Description

Show user that he can not drop on toolbar/ui on dragover.

Change History (11)

comment:1 Changed 4 years ago by Piotr Jasiun

Status: newconfirmed

comment:2 Changed 4 years ago by Artur Delura

How about letting user know where they can drop files, just like others apps do? For example gmail, facebook.

It's quite straightforward that we can't drop objects on toolbar, but toolbar is part of editor. But other page elements are not ours, and we won't show that user can't drop elements there. Showing that user can't drop on toolbar and not showing that he can't drop somewhere else on page is inconsistent.

comment:3 Changed 4 years ago by Piotr Jasiun

We were talking a lot about the UX of the upload feature, twice. And since I really like the way Gmail or Facebook works, we decided that this way user will not be able to set cursor in the position he wants, so we will not add any drop area. We also decided that will just show that he can not drop into toolbar and we will not reload page if he do so. Lets not start this discussion for the third time.

comment:4 Changed 4 years ago by Marek Lewandowski

Yes, the thing with preventing page reload is super important.

Since we're supporting dnd just imagine how terrible it was if user just accidentally pulled his cursor over toolbar before the drop. It might result with losing unsaved content.

comment:5 Changed 4 years ago by Artur Delura

I've implemented preventing page reload when user drop image on toolbar and bottom in all browsers. Also implemented showing user that they can't drop there by appropriate cursor. The problem with cursor exist in IE:

To set a proper cursor we need to manipulate dropEffect and effectAllowed dataTransfer object property. In all browsers expect IE it's enought to get effect by only manipulating only dropEffect. According to this article (section remarks) to make things work on IE we have to manipulate effectAllowed as well in dragstart event. Unfortunatelly this event won't fire on any browser if we drag files outside the browser scope (system file application). So we are not able to get an effect on IE.

I also tried to add special class no-drop which set CSS cursor property to editor container while user drag over, but browser doesn't respect it. This is quite logical.

If we want to let user know that they can't drop somewhere we can still manipulate other CSS properties which will bring visual effect. Below I attached code which is responsible for adding such class if we decide so.

var enter = 0,
	top = editor.ui.space( 'top' ),
	bottom = editor.ui.space( 'bottom' );

editor.container.on( 'dragleave', function() {
	if ( --enter === 0 ) {
		top.removeClass( 'no-drop' );
		bottom.removeClass( 'no-drop' );
	}
} );

editor.container.on( 'dragenter', function( evt ) {
	if ( enter++ === 0 ) {
		top.addClass( 'no-drop' );
		bottom.addClass( 'no-drop' );
	}
	evt.data.preventDefault();
} );

Gmail and Evernote apps doesn't show no-drop cursor in IE as well. Facebook doesn't care in any browser :D

comment:6 Changed 4 years ago by Artur Delura

Owner: set to Artur Delura
Status: confirmedreview

Changes in branch:t/12613.

comment:7 Changed 4 years ago by Piotr Jasiun

Does on 'drop' listener is needed? I removed it and everything seems to work fine. And less we change the page behavior, the better.

Last edited 4 years ago by Piotr Jasiun (previous) (diff)

comment:8 Changed 4 years ago by Piotr Jasiun

Status: reviewreview_failed

These changes break drag and drop in inline editor.

comment:9 Changed 4 years ago by Artur Delura

Right - we don't need this. I also added dragover event to more specific elements.

comment:10 Changed 4 years ago by Artur Delura

Status: review_failedreview

Changes in the same branch.

comment:11 Changed 4 years ago by Piotr Jasiun

Resolution: fixed
Status: reviewclosed

Closed with git:5394b5b.

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy