Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: RTE misplaced when opened right after dropping activeOnRender component #3579

Closed
2 of 4 tasks
mcottret opened this issue Jun 30, 2021 · 6 comments
Closed
2 of 4 tasks

Comments

@mcottret
Copy link

mcottret commented Jun 30, 2021

Version: 0.17.4

Are you able to reproduce the bug from the demo?

  • Yes
  • No

Steps to reproduce:

  • Go to the demo
  • Drop a "Text" block inside the canvas
  • Click on the "Text" component that was just dropped
  • The opened RTE is misplaced (cf attached screenshot)

What is the expected behavior?

The opened RTE should be positioned correctly

What is the current behavior?

The RTE is misplaced, it hides the "Text" component's content & can prevent editing properly (especially when using bigger custom RTE like CKEditor).

This only happens when the RTE is first opened & only if the dropped block is a "Text" component with activeOnRender option set to true.

This is caused by a miscalculation of the toolbar's position when its offsetHeight equals 0. Itself due to its parent (CanvasView.toolsEl) being set to display: none at this time (because rte.enable is called on drop when activeOnRender is true, which happens while the component is unselected as opposed to normal, hence its toolsEl still being set to display: none)

Proposed solution:

Selecting the component before triggering activeOnRender's active event seems to fix the issue.

Concretely, adding em.setSelected(result); before result.trigger('active'); here should do it, unless I'm missing something.

I could take care of the PR if this looks good to you :)

Are you able to attach screenshots, screencasts or a live demo?

  • Yes (attach)
  • No

Screenshot 2021-06-30 at 4 21 11 PM

@artf
Copy link
Member

artf commented Jul 22, 2021

Yes, I guess you're right. It doesn't actually make sense activating RTE without selecting the component (it probably makes sense with all other "activatable" blocks).

@artf artf added the bug label Jul 22, 2021
@artf artf added this to To do in Release v0.17.22 via automation Jul 22, 2021
@artf artf closed this as completed in bb4a661 Jul 22, 2021
Release v0.17.22 automation moved this from To do to Done Jul 22, 2021
@ronaldohoch
Copy link

ronaldohoch commented Aug 17, 2021

Hello, just updated the file from commit bb4a661, and i think it's so close, i'm using the grapesjs-ckeditor code and the first open of RTE, i'ts placed right:
image

But at the second time the rte is open, it's misplaced again... :/
image

I'll keep checking for it :/

The code i'm get from grapesjs-ckeditor
      let c = {
        options:{
          sharedSpaces:''
        }
      };

      let defaults = {
        // CKEditor options
        options: {},

        // On which side of the element to position the toolbar
        // Available options: 'left|center|right'
        position: 'left',
      };

      // Load defaults
      for (let name in defaults) {
        if (!(name in c))
          c[name] = defaults[name];
      }

      if (!CKEDITOR) {
        throw new Error('CKEDITOR instance not found');
      }

      editor.setCustomRte({
        enable(el, rte) {
          // If already exists I'll just focus on it
          if(rte && rte.status != 'destroyed') {
            this.focus(el, rte);
            return rte;
          }

          el.contentEditable = true;

          // Seems like 'sharedspace' plugin doesn't work exactly as expected
          // so will help hiding other toolbars already created
          let rteToolbar = editor.RichTextEditor.getToolbarEl();
          [].forEach.call(rteToolbar.children, (child) => {
            child.style.display = 'none';
          });

          // Check for the mandatory options
          var opt = c.options;
          var plgName = 'sharedspace';

          if (opt.extraPlugins) {
            if (typeof opt.extraPlugins === 'string')
              opt.extraPlugins += ',' + plgName;
            else
              opt.extraPlugins.push(plgName);
          } else {
            opt.extraPlugins = plgName;
          }

          if(!c.options.sharedSpaces) {
            c.options.sharedSpaces = {top: rteToolbar};
          }

          // Init CkEditors
          rte = CKEDITOR.inline(el, c.options);

          /**
           * Implement the `rte.getContent` method so that GrapesJS is able to retrieve CKE's generated content (`rte.getData`) properly
           *
           * See:
           *  - {@link https://github.com/artf/grapesjs/issues/2916}
           *  - {@link https://github.com/artf/grapesjs/blob/dev/src/dom_components/view/ComponentTextView.js#L80}
           *  - {@link https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_editor.html#method-getData}
           */
          rte.getContent = rte.getData;

          // Make click event propogate
          rte.on('contentDom', () => {
            var editable = rte.editable();
            editable.attachListener(editable, 'click', () => {
              el.click();
            });
          });

          // The toolbar is not immediatly loaded so will be wrong positioned.
          // With this trick we trigger an event which updates the toolbar position
          rte.on('instanceReady', e => {
            var toolbar = rteToolbar.querySelector('#cke_' + rte.name);
            if (toolbar) {
              toolbar.style.display = 'block';
            }
            editor.trigger('canvasScroll')
          });

          // Prevent blur when some of CKEditor's element is clicked
          rte.on('dialogShow', e => {
            const editorEls = grapesjs.$('.cke_dialog_background_cover, .cke_dialog');
            ['off', 'on'].forEach(m => editorEls[m]('mousedown', stopPropagation));
          });

          this.focus(el, rte);


          return rte;
        },

        disable(el, rte) {
          el.contentEditable = false;
          if(rte && rte.focusManager)
            rte.focusManager.blur(true);
        },

        focus(el, rte) {
          // Do nothing if already focused
          if (rte && rte.focusManager.hasFocus) {
            return;
          }
          el.contentEditable = true;
          rte && rte.focus();
        },
      });

      // Update RTE toolbar position
      editor.on('rteToolbarPosUpdate', (pos) => {
        // Update by position
        switch (c.position) {
          case 'center':
            let diff = (pos.elementWidth / 2) - (pos.targetWidth / 2);
            pos.left = pos.elementLeft + diff;
            break;
          case 'right':
            let width = pos.targetWidth;
            pos.left = pos.elementLeft + pos.elementWidth - width;
            break;
        }

        if (pos.top <= pos.canvasTop) {
          pos.top = pos.elementTop + pos.elementHeight;
        }

        // Check if not outside of the canvas
        if (pos.left < pos.canvasLeft) {
          pos.left = pos.canvasLeft;
        }
      });

@migokcek
Copy link

I have the same issue. Did anyone find a solution?

@gustavohleal
Copy link

Hi. Me and @ronaldohoch have found a workaround for this problem.

We trigger the scroll event when the RTE is enabled.
Here's the code:

    // Trigger scroll event from canvas so that grapesjs
    // fix CKEditor position correctly
    editor.on("rte:enable", () => {
        editor.trigger('canvasScroll');
    });

@migokcek
Copy link

Thank you for your answer, i tried your code after
const editor = grapesjs.init({...})
But it didn't change anything, i thought it's about my grapesjs version and, i upgraded my version but still missplaced when i open the text block for the first time

@ronaldohoch
Copy link

Can you provide some code in jsfiddle? https://jsfiddle.net/szLp8h4n

felixx-kreebox pushed a commit to kreeboxvn/grapesjs that referenced this issue Feb 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

No branches or pull requests

5 participants