search

silverstripe / silverstripe-elemental

Create pages in Silverstripe CMS using content blocks
http://dna.co.nz
BSD 3-Clause "New" or "Revised" License
110 stars 115 forks source link

Dragging a block while open removes the visible content in TinyMCE #692

Open clarkepaul opened 5 years ago

clarkepaul commented 5 years ago

After dragging an open content block with a TinyMCE field the editor appears empty. If you refresh the page the content is actually still there but is concerning to users especially if they wish to continue editing the content or think it's been deleted. SS4.4

NightJar commented 5 years ago

Steps to reproduce:

  1. create a Blocks Page
  2. create 5 content elements on this page, using data from https://lipsum.com to fill in the content
  3. Save the page
  4. Drag the last element to be the first element

Observe: elements 2-5 TinyMCE contents are now blank. The dragged element's content is still visible.

The content disappears for all "inbetween" elements, e.g. moving the 4th element to be 2nd and only the 3rd element (the old 2nd) will lose data. ... Actually, this is limited to the elements that move down. If I move e.g. the 2nd element to be 3rd, it is the element that was moved that loses its TinyMCE data.

I've inspected the Redux state tree, and the correct data is still present in the redux form shard (e.g. form.formState.element.ElementForm_1.values.PageElements_1_HTML)

What I've not confirmed (yet) though is whether that ID is still correct (i.e. is it the order that changes the key (ElementForm_1), or the ID of the backing model? 🤔 ... This appears not to be the case. The ID is a prop that follows the element, and one would expect that if the content should disappear for this reason, then so would the title data (which is still present and correct).

Collapsing and re-expanding the afflicted inline form will also see the data re-appear in the rich text editor.

Just found that expanding and collapsing elements enough appears to remove the entire editor from my Page's edit form. So that's fun. Will open a new issue if I can recreate.

ScopeyNZ commented 5 years ago

Is that the summary of the content or the content that appears within the form for each element?

NightJar commented 5 years ago

Is that the summary of the content or the content that appears within the form for each element?

The inline editable TinyMCE content :)

This appears to be an issue with TinyMCE and perhaps the way it binds functions to the DOM, upon moving the DOM it is bound to, the method binding is lost, and ceases to function as intended.

I investigated ways to get TinyMCE to refresh itself, but in the end it seems the only way to solve is to destroy the instance and recreate it. Luckily we already wrap the TinyMCE instance so do not lose the settings it was created with after destroying - recration is easy enough, see https://github.com/silverstripe/silverstripe-admin/pull/948

As to getting the element list to trigger this functionality, a lot of investigation has gone into the drag and drop lib, searching for an appropriate place to trigger the refresh after a drop event. However in then end (thanks to @ScopeyNZ for some peer programming) the easiest way is actually to simply hook the list component, as the drop result will update the list of elements it displays.

The remaining concern is with limiting this expensive operation to only those elements that need it, as opposed to all TinyMCE instances on a page (which may be numerous), where a flickering of which may cause discombobulation for the editor.

This is implemented as a hack in ElementList.componentDidUpdate, and a work in progress branch can be found at https://github.com/creative-commoners/silverstripe-elemental/tree/pulls/4.1/undisappear-tinymce - the work is close to complete I think, but not finished as it causes errors with React, destroying the entire elemental editor.

robbieaverill commented 5 years ago

https://stackoverflow.com/questions/2535569/tinymce-editor-dislikes-being-moved-around is a good reference to why this is an issue

NightJar commented 5 years ago

Nice find. Weird it only happens when moving "down" the DOM tree though 😕 🤷‍♂

robbieaverill commented 5 years ago

I'm suggesting we park this issue for now

brynwhyman commented 5 years ago

If this was parked, where would that leave https://github.com/silverstripe/silverstripe-admin/pull/948?

NightJar commented 5 years ago

That's able to be merged, as it doesn't actually do what the title says (my fault), but rather indicates it's role to play in this fix, which wasn't actually feasible in the end.

I've changed the title on that PR.

brynwhyman commented 5 years ago

Nice one. Thanks NightJar