Opened 5 years ago

Closed 5 years ago

#12169 closed New Feature (fixed)

Drop events.

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

Description

Part of #11437.

Editor should emit drag & drop events with data transfer facade.

Change History (17)

comment:1 Changed 5 years ago by Piotr Jasiun

Type: BugNew Feature

comment:2 Changed 5 years ago by Piotrek Koszuliński

Resolution: invalid
Status: newclosed

We don't need editor#drop (as agreed in #11437) and we don't see yet need for editor#drag.

comment:3 Changed 5 years ago by Piotr Jasiun

Resolution: invalid
Status: closedreopened

We decided that we need such facade.

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

comment:4 Changed 5 years ago by Jakub Ś

Status: reopenedconfirmed

comment:5 Changed 5 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: confirmedassigned

comment:6 Changed 5 years ago by Piotr Jasiun

Milestone: CKEditor 4.5.0

comment:7 Changed 5 years ago by Piotr Jasiun

Status: assignedreview

Code, docs and tests done. Branch in based on #12168 because both tickets deeply modify tests structure so merge could be inconvenient. Changes in t/12169.

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

comment:8 Changed 5 years ago by Piotr Jasiun

Rebased on newest #12168.

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

comment:9 Changed 5 years ago by Piotr Jasiun

Rebased on newest #12168.

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

comment:10 Changed 5 years ago by Piotr Jasiun

Rebased on major.

comment:11 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_failed

comment:12 in reply to:  11 Changed 5 years ago by Piotr Jasiun

Status: review_failedreview

Fixed.

About:

I cannot use 'afterPaste', because this is a case when there is no paste nor afterPaste events and I need to wait to check if no paste event were fired. But I decreased timeout from 300 to 0.

comment:13 Changed 5 years ago by Piotrek Koszuliński

I cannot use 'afterPaste', because this is a case when there is no paste nor afterPaste events and I need to wait to check if no paste event were fired. But I decreased timeout from 300 to 0.

Hmm... Yeah, so this is more problematic. You can verify that 0ms is enough by breaking code and checking if test failed. If you used TDD you would be sure ;)

comment:14 Changed 5 years ago by Piotr Jasiun

I set expectedPasteEventCount to 0 and all tests which fire paste event were red, so timeout 0 is not too small (paste was fired before event count was checked).

This should be checked again for #12173.

comment:15 in reply to:  14 Changed 5 years ago by Piotrek Koszuliński

Replying to pjasiun:

I set expectedPasteEventCount to 0 and all tests which fire paste event were red, so timeout 0 is not too small (paste was fired before event count was checked).

I rather meant breaking plugin's code, not tests. You may not predict that in case in which event is cancelled paste event may be fired differently. That's why you should break the place where a decision is made whether to continue firing events process. That's why TDD is much better, because you first see the test failing.

comment:16 Changed 5 years ago by Piotrek Koszuliński

Status: reviewreview_passed

comment:17 Changed 5 years ago by Piotr Jasiun

Resolution: fixed
Status: review_passedclosed

I change if the change in the code causes red test and it does.

Majorized with hash: git:1bb7369.

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