GrapesJS / grapesjs

Free and Open source Web Builder Framework. Next generation tool for building templates without coding
https://grapesjs.com
BSD 3-Clause "New" or "Revised" License
22.37k stars 4.05k forks source link

BUG: update() of custom types added to StyleManager is no more triggered when switching between some targets. #4601

Closed bit-ocean closed 2 years ago

bit-ocean commented 2 years ago

GrapesJS version

What browser are you using?

Chromium 104

Reproducible demo link

https://jsfiddle.net/aw8p0h9j/

Describe the bug

How to reproduce the bug?

  1. Open the jsfiddle link.
  2. Click between the divs, then between any of divs and the background.

What is the expected behavior? In older versions of grapesjs (0.17.26), the update is triggered and console logs the clicks, the code of update() is executed.

What is the current behavior? In the version 0.20.1 of grapesjs, the update is not triggered when switching between different targets at least when 1. no property value on component, 2. value on components is the same, ...

JS:

const editor = grapesjs.init({
    container: '#gjs',
  fromElement: 1,
  height: '100%',
  styleManager: {},
  storageManager: { type: 0 },
  plugins: ['gjs-blocks-basic', ]
});

  const sm = editor.StyleManager;

  sm.addType('newinput', {
     // Create UI
     create({ props, change }) {
       const el = document.createElement('div');
       el.innerHTML = '<input class="my-input" />';
       const inputEl = el.querySelector('.my-input');
       inputEl.addEventListener('change', event => change({ event }));
       inputEl.addEventListener('input', event => change({ event, partial: true }));
       return el;
     },
     // Propagate UI changes up to the targets
     emit({ props, updateStyle }, { event, partial }) {
       console.log("Change in value.");
     },
     // Update UI (eg. when the target is changed)
     update({ value, el }) {
       console.log("Target changed", Date.now());
     },
     // Clean the memory from side effects if necessary (eg. global event listeners, etc.)
     destroy() {}
  })

  // Add new sector
  const newSector = sm.addSector('sector-id', {
    name: 'New sector',
    open: true,
    properties: [
        {
        name: "Custom property",
        type: "newinput",
        property: "padding",
      }
    ],
  });

HTML:

<div id="gjs">
  <div style="padding: 25px">Logs appear only when clicking between body and any of divs, but not between divs.</div>
  <div style="padding: 25px">As long the divs have the same value of padding there is no log while clicking between them.</div>
</div>

Is this intended breaking change or a bug?

Code of Conduct

ronaldohoch commented 2 years ago

Same as described here, not a issue: https://github.com/artf/grapesjs/issues/4350

artf commented 2 years ago

Yeah but even in a case of a custom UI, the update is skipped if the value is not changed as it's not necessary.

bit-ocean commented 2 years ago

The code that I pasted is using the standard api, pretty much copy/paste from the documentation. And clearly the update's mechanism has changed between the versions.

Before if target element had been changed, the update was triggered, and we could run code that under certain conditions re-renders the component. @artf what would be your proposal of best strategy to replace it now? Should I add a custom listener to target change event rather? What is your recommendation of implementation that's gonna ensure more compatibility with staying up to date with core changes of grapesjs?

In short, I need to be able, when target is changed, to call a callable and both get and set the value of the custom type input.

artf commented 2 years ago

From the API perspective, it doesn't make sense to update the custom UI elements if the value doesn't change, so what you're asking is out of scope. Yes, you can try to retrigger the update of your custom UI on component select change (eg. during create). If you need more control over your UI probably you should go with the custom Style Manager option