[CKEditor Dev] #10280: Replace textarea with inline editor
CKEditor
-------------------------+-----------------
Reporter: fredck | Owner:
Type: New Feature | Status: new
Priority: Normal | Milestone:
Component: General | Version:
Keywords: |
-------------------------+-----------------
The idea is introducing a new instance creation method which is similar to
CKEDITOR.replace, but instead of having a themed-ui editor instance
created, CKEditor should replace the <textarea> with a <div>, created on
the fly, enabling inline editing on it.
The created <div> must have standard class names available, so
implementors can customize it's appearance.
Then, when the form is posted, the original textarea is filled with the
editor contents, just like it happens with CKEDITOR.replace.
CKEDITOR.destroy should revert it back to the textarea, just like
CKEDITOR.replace as well.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280>
CKEditor <http://ckeditor.com/>
The text editor for the Internet
CKEditor
Reporter: fredck | Owner:
Type: New Feature | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
Changes (by fredck):
* cc: Senthil (added)
* keywords: => Oracle
* status: new => confirmed
* milestone: => CKEditor 4.2
Comment:
We can tentatively include this in CKEditor 4.2.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:1>
CKEditor
Reporter: fredck | Owner:
Type: New Feature | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
* cc: ramesh.ponnusamy@… (added)
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:2>
CKEditor
Reporter: fredck | Owner:
Type: New Feature | Status: confirmed
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
Comment (by Reinmar):
> The created <div> must have standard class names available, so
implementors can customize it's appearance.
I think that these classes should be configurable, just like they
**should** be configurable for a divarea, which we cannot style right now.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:3>
CKEditor
Reporter: fredck | Owner: m.guzek
Type: New Feature | Status: assigned
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
* owner: => m.guzek
* status: confirmed => assigned
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:4>
CKEditor
Reporter: fredck | Owner: m.guzek
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
Changes (by m.guzek):
* status: assigned => review
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:5>
CKEditor
Reporter: fredck | Owner: m.guzek
Type: New Feature | Status: review_failed
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
Changes (by Reinmar):
* status: review => review_failed
Comment:
* Many tests are not passing.
* Tests for this feature should be placed in `dt/` (they are design
tests).
* Code style... huge number of issues in every file.
* There are no tests for:
* editor initialization with string (id),
* what happens after `destroy()` (whether everything is cleaned up,
whether element is updated, etc),
* what happens on form submit,
* for editor.attachToForm,
* whether textarea is hidden,
* "fix the readonly setting" for this comment from dev code,
* From tests: `this.editor1.instanceReady`. What is it?
* We don't need all these plugins in those tests.
* There's no docs for attachToForm and it is not clear for which editor
this method should be executed, for which can be. Perhaps this should be
private.
* Why do we need changes in `core/editable.js`?
* Missing sample.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:6>
CKEditor
Reporter: fredck | Owner: m.guzek
Type: New Feature | Status: review
Component: General | Version:
Resolution: | Keywords: Oracle
Changes (by m.guzek):
* status: review_failed => review
Comment:
sample added to samples/inlinebycode.html. Additional tests added. Code
style corrected. Ready for next review.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:7>
CKEditor
Reporter: fredck | Owner: m.guzek
Type: New Feature | Status: review_failed
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
Changes (by a.nowodzinski):
* status: review => review_failed
Comment:
I pushed several commits both in '''dev''' and '''test''' branch. I fixed
lots of issues, enhanced tests and refactored the code. For more details,
please refer to commit descriptions. The most notable are:
* Test in `core/creators/creators.html`
* Tests in `core/creators/inline-textarea.html`
* Test in `core/editor/readonly.html`. While extending the
`editor.readOnly` discovery I noticed that `element.getAttribute` is used
to determine `disabled` attribute value. It's incorrect since `disabled`
doesn't require any value to make an element read-only like:
{{{
// It's disabled but element.getAttribute( 'disabled' ) is empty string.
<textarea disabled>Foo</textarea>
}}}
I switched the condition to `hasAttibute( 'disabled' )` and created tests
for all available creators to make sure this is correct.
There are still several things that make this branch R-:
* Two tests in `inline-textarea.html` fail. It's possible to create a new
instance with `CKEDITOR.inline()` on the container used by another inline-
textarea editor.
* Missing tests for form submit.
* Missing tests for `editor.attachToForm`.
* Which editors can use it?
* Why?
* Does de-attachment work on editor destroy?
* Most likely in `core/editor/form.html`
* Missing docs for the new feature.
* For `CKEDITOR.inline`
* Some others?
* We should remember to update guides as well.
* Sample needs tuning.
* It doesn't clearly say that it's possible to create editor from
`<textarea>`.
* There's no sample code in the description.
* Two cases (contenteditable=true and textarea) must be clearly separated
from each other. Example is `samples/magicline.html` where two editors
have different descriptions (`.description`) and share the introduction.
Each editor should have own sample code.
Other remarks (already fixed):
* `editor.setData` is async (see docs)
* We don't use `editor.instanceReady` to determine the status. Event
callback is the right place.
* `document.getElementById` is `CKEDITOR.document.getById`
* We optimize the code
{{{
assert.isTrue( document.getElementById( 'editor1' ).offsetWidth == 0 );
assert.isTrue( document.getElementById( 'editor1' ).offsetHeight == 0 );
// another getElementById call
}}}
{{{
editor.on( 'destroy', function() { // editor is in the outer scope
var editor = this, // Why?
...
editor.element.clearCustomData();
...
element.clearCustomData(); // Why again?
...
});
}}}
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:8>
CKEditor
Reporter: fredck | Owner: m.guzek
Type: New Feature | Status: review_failed
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+---------------------------
Comment (by Reinmar):
Comment regarding this line: https://github.com/cksource/ckeditor-
dev/commit/2675b477953058660386128669e8a0804fd6452c#L0R42
IMO it could be better to first create that div and then set its
innerHTML. This way we would be secure in case of corrupted data. It won't
be possible that data splits contenteditable div into more elements (e.g.
when this is loaded '</div>').
On the other hand - only the moment between inline() call and the final
#loaded (when processed&fixed data is set back to editable div) would be
influenced by not fixing this.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:9>
CKEditor
Reporter: fredck | Owner: pjasiun
Type: New Feature | Status: assigned
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
* status: review_failed => assigned
* owner: m.guzek => pjasiun
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:10>
CKEditor
Reporter: fredck | Owner: pjasiun
Type: New Feature | Status: assigned
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
Comment (by pjasiun):
Replying to [comment:8 a.nowodzinski]:
> There are still several things that make this branch R-:
>
> * Two tests in `inline-textarea.html` fail. It's possible to create a
new instance with `CKEDITOR.inline()` on the container used by another
inline-textarea editor.
After talk with Fred and PK I agree that there is no reason to check this.
Container is internal `div` created by JS method. We don't check if it is
possible to create Editor on any other `div` created using
`replace`/`inline`.
> * Missing tests for form submit.
I'm working on this.
> * Missing tests for `editor.attachToForm`.
> * Which editors can use it?
> * Why?
> * Does de-attachment work on editor destroy?
> * Most likely in `core/editor/form.html`
`editor.attachToForm` is now private. I also added docs to this method.
tests for form submit will check if `attachToForm` works so we don't need
separate tests for that.
> * Missing docs for the new feature.
> * For `CKEDITOR.inline`
> * Some others?
> * We should remember to update guides as well.
> * Sample needs tuning.
> * It doesn't clearly say that it's possible to create editor from
`<textarea>`.
> * There's no sample code in the description.
> * Two cases (contenteditable=true and textarea) must be clearly
separated from each other. Example is `samples/magicline.html` where two
editors have different descriptions (`.description`) and share the
introduction. Each editor should have own sample code.
I will do this.
I also added class `cke_textarea_inline` to div created inside `inline`.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:11>
CKEditor
Reporter: fredck | Owner: pjasiun
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
Resolution: | Keywords: Oracle
--------------------------+--------------------------
* status: assigned => review
Comment:
Test for submit are done. I've also made small refactoring and added
documetation to inline method.
I will work now on sample with is not ready yet, but I put it in review to
make thinks faster.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:12>
CKEditor
Reporter: fredck | Owner: pjasiun
Priority: Normal | Milestone: CKEditor 4.2
Component: General | Version:
--------------------------+--------------------------
Changes (by Reinmar):
* status: review_passed => closed
* resolution: => fixed
Comment:
Merged to major with git:d20c978 on dev and c00a6d5 on tests.
--
Ticket URL: <http://dev.ckeditor.com/ticket/10280#comment:14>