Edit multiple elements - previous actions are assign to last

[marczuk]

Describe the bug

I have few texts to edit on one page. When I'm trying to edit few of them at once. And click on appeared buttons, doesn't matter on which button I click, they will execute only on last one expanded.

Steps to reproduce

My malle setup:

// now create the malle
const malle = new Malle({
  debug: true,
  formClasses: ['d-flex'],
  cancel: 'Cancel',
  cancelClasses: ['btn', 'btn-danger', 'btn-sm'],
  inputClasses: ['form-control', 'd-inline'],
  submit: 'OK',
  submitClasses: [ 'btn', 'btn-primary', 'mx-2', 'btn-sm'],
  tooltip: 'Click this to edit',
  onBlur: Action.Ignore,
  onEnter: Action.Ignore,
  // this is the function that will be called once user has finished entering text (press Enter or click outside)
  // it receives the new value, the original element, the event and the input element
  fun: (value, original, event, input) => {
    let fieldName = original.getAttribute('id')
    return myCustomFunction(value, fieldName);
  },
}).listen(); // directly start listening after object creation

and later in html:

<label for="testName" class="form-label">Name</label>
<span type="text" id="testName" data-malleable='true' class="form-control">{{ test['testName'] }}</span>

<br />
<label for="testBranch" class="form-label">Branch</label>
<span type="text" id="testBranch" data-malleable='true' class="form-control">{{ test['testBranch'] }}</span>

Information

• malle version: latest 2.2.0 "@deltablot/malle": "^2.2.0"
• Client operating system (e.g: MacOS, Windows, GNU+Linux): MacOS
• Browser (e.g. firefox, chrome): Chrome

[NicolasCARPi]

When I'm trying to edit few of them at once.

You mean having several inputs opened at the same time?

[marczuk]

Yes

[NicolasCARPi]

I haven't looked into it, but it was never designed to have multiple inputs opened at the same time, so I'm not surprised that you get this undefined behavior.

[NicolasCARPi]

I've looked into it but didn't find a solution to this behavior, so I'm open to suggestions, or any help on that matter.

[marczuk]

I was thinking about two possible solutions:

1. pro solution - to check during save/cancel operations the 'id' of the editable element or just search for a parent.
2. as a fast solution - to just lock rest of the editable fields when one is edited at this moment. You have to save to cancell one to edit another.

[NicolasCARPi]

I think this issue is that there is only one instance of Malle, and it listens on all elements, and this leads to this bug.

So one approach to solve this issue would be to have one instance per target, it's not like Malle has a huge memory footprint. And this you can do already by having one instance listen to only one element.
Downside is that it might mean a lot of boilerplate code to duplicate, and you need to write a new Malle() for each element, which doesn't work well with dynamic elements.

And then we can have your second point, that if a Malle instance listens on more than one element, then editing another one is not permitted while one is "opened", and it shows an error message to that effect.

So the better solution would be to handle better multiple opened elements. You suggested looking at an id or parentElement, but this won't work 100% of the time. I'm thinking about storing some sort of internal identifier (counter) for elements, but I'll have to get back to this and take the time to do it right. Not sure if it's a good strategy anyway...

[NicolasCARPi]

@marczuk Can you try the branch "multi" please? It should solve this issue. FYI, here is the diff:

diff --git a/src/main.ts b/src/main.ts
index 982979a..451c6db 100644
--- a/src/main.ts
+++ b/src/main.ts
@@ -127,7 +127,10 @@ export class Malle {
   listen() {
     document.querySelectorAll(this.opt.listenOn)
       .forEach((el: HTMLElement) => {
-        el.addEventListener(this.opt.event, this.process.bind(this));
+        const opt = this.opt;
+        opt.listenNow = false;
+        const m = new Malle(opt);
+        el.addEventListener(this.opt.event, m.process.bind(m));
         // make the mouse change to pointer on targeted elements
         el.style.cursor = 'pointer';
         if (this.opt.tooltip) {

[marczuk]

I just tried, but I'm not sure why, but when executing: npm i @deltablot/malle@deltablot/malle#multi
I'm getting malle without dist catalog.

[NicolasCARPi]

Yes it won't work like this as the dist file is built by the publish action, you'll need to pull the branch, run npm run build and copy the dist content over to your node_modules folder.

[marczuk]

works perfectly now :) waiting for a release :)