GetmeUK / ContentTools

A JS library for building WYSIWYG editors for HTML content.
http://getcontenttools.com
MIT License
3.95k stars 395 forks source link

Bug: Clicking from one empty element(with placeholder) into another creates duplicate element when using Undo #475

Open scmccoy opened 6 years ago

scmccoy commented 6 years ago

Hello - I am having an issue with the Placeholder solution presented in #275. It works great except in the case of using the Undo button and having an empty element. This may be a bug. To Replicate:

My code for the createPlaceholderElement function:

    _EditorApp.prototype.createPlaceholderElement = function(region) {
      var text, type;
      text = region._domElement.getAttribute('data-ce-placeholder') || 'Enter content here';
      type = region._domElement.getAttribute('data-placeholder-type') || 'p';
      return new ContentEdit.Text(type, {
        'data-ce-placeholder': text
      }, '');
    };

My HTML:

    <div ng-editable data-placeholder-type="h3" data-ce-placeholder="Art Event"></div>
    <div ng-editable data-placeholder-type="h1" data-ce-placeholder="Name of Event"></div>

It looks like the _preventEmptyRegions function is adding a single placeholder of the element I am clicking into here:

        placeholder = this.createPlaceholderElement(region);
        region.attach(placeholder);

then adding both elements placeholders in _EditorApp.prototype.syncRegions function at this line:

      if (this._state === 'editing') {
        this._initRegions(restoring);
        this._preventEmptyRegions();
      }

I haven't figured out a solution to prevent the doubling up of elements. Any advice would be great, thanks! Shane pixelsnap 2018-02-14 at 17 40 36 pixelsnap 2018-02-14 at 17 40 57

anthonyjb commented 6 years ago

@scmccoy yeah this looks like a bug to me, thanks for flagging the problem in detail :+1: - have labelled as such and will take a look at resolving this weekend.

scmccoy commented 6 years ago

Hi Anthony, I just had a chance to test a little more - my current work around is checking in _preventEmptyRegions function if the children length is greater than or equal to 1. (around line 8855)

placeholder = this.createPlaceholderElement(region); if(region._domElement.children.length >= 1) { // do nothing } else { region.attach(placeholder); } _results.push(region._modified = lastModified);

On the second run of the function the length is 2 and skips the region.attach(placeholder). Thoughts? I am not sure how this may affect other areas. Thank you for your time, Shane M.