Opened 10 years ago

Closed 10 years ago

#12168 closed New Feature (fixed)

Improve dataTransfer object

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

Description (last modified by Piotr Jasiun)

Part of #11437.

setData and getData for custom data types should work on every browser. dataTransfer object should also have isDrag property.

Change History (12)

comment:1 Changed 10 years ago by Piotr Jasiun

Description: modified (diff)

comment:2 Changed 10 years ago by Piotr Jasiun

Owner: set to Piotr Jasiun
Status: newassigned

comment:3 Changed 10 years ago by Piotr Jasiun

Description: modified (diff)

comment:4 Changed 10 years ago by Piotr Jasiun

  • reorganized event data and dataTransfer structure so now it looks like this:
    {
      dataValue:            string,
      type:                 'auto|text|html',
      [method]:               'drop|undefined',
      [dataTransfer]: {
          id:               string,
          sourceEditor:     editor,
          targetEditor:     editor,
          getData:          function,
          setData:          function,
          getTransferType:  function, // returns external|internal|cross editor
      }
    }
    
  • improved setData and getData so now they support custom data types on every browser,
  • dataTransfer may have no native object,
  • used code from insertText to transform text to html instead of simple htmlEncode.

Now dataVaule and type are set, based on dataTransfer data, just before firing paste. It will be moved to the paste listener in #12173, but now paste event is not firing if dataValue is empty so we need to set it earlier.

Changes in t/12168.

comment:5 Changed 10 years ago by Piotr Jasiun

Status: assignedreview

comment:6 Changed 10 years ago by Piotrek Koszuliński

Status: reviewreview_failed

comment:7 Changed 10 years ago by Piotr Jasiun

Status: review_failedreview

Changes done and pushed.

transformPlainTextToHtml needs only enterMode as an option so I decided no to create option object for one option. In the future if we will have more options we can check the type of the parameter.

dataTransfer.setData will always work, even if native method will not, so it is pointless to return result.

comment:8 Changed 10 years ago by Piotr Jasiun

Rebased on newest major.

comment:9 Changed 10 years ago by Piotrek Koszuliński

Status: reviewreview_failed
  1. You forgot to remove editable#textToHtml method.
  2. I see that the code which decides which enter mode should be used while transforming text o HTML is repeated twice - in clipboard plugin and in editable. This means that we need editable#transformPlainTextToHtml which will do this and which will use tools.transformPlainTextToHtml.
  3. You forgot the @since tags.
  4. https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L382 - don't mention paste position, because you don't know context in which this method will be used - it'd a general purpose transform function.
  5. https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L383 - "Transformed" indicates that this is the HTML that has been transformed, while, in real, it was generated.
  6. https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L383 - start comment from the upper case.
  7. https://github.com/cksource/ckeditor-dev/blob/de348e9ded089a5f8111d127d71eed7096c3bd40/core/tools.js#L467 - unnecessary semicolon. Actually, there are 5 linter errors.
  8. tools#getUniqueId does not have tests. Please write at least one that will verify e.g. type of returned value and its length.

If any of these points conflicts with code created in branches based we can extract it to a separate ticket and fix afterwards, because none is critical.

comment:10 Changed 10 years ago by Piotr Jasiun

Status: review_failedreview

Changes done and pushed to t/12168.

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

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

Status: reviewreview_passed

comment:12 Changed 10 years ago by Piotr Jasiun

Resolution: fixed
Status: review_passedclosed

Closed with hash git:cb28e9c.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy