I've a problem with my binding event, I would like to remove each event when change event is fired. It's working fine without a loop.
this.objectInstance.on('selected', () => {
document.querySelectorAll('select').forEach((el) => {
this.test = this.fontFamilyHandler.bind(this, el.getAttribute('id'));
el.addEventListener('change', this.test);
})
});
this.objectInstance.on('deselected', () => {
document.querySelectorAll('select').forEach((el) => {
el.removeEventListener('change', this.test);
})
});
fontFamilyHandler = (key, evt) => {
this.objectInstance.set(key, evt.target.value);
this.canvas.requestRenderAll();
}
Do you have any idea why it's doesn't work ?
In every iteration, you are overwriting this.test with the next listener. So at the end, this.test will contain the last listener attached and therefore it can only remove that one.
I can see two solutions here:
this.objectInstance.on('selected', () => {
this.test = new WeakMap();
document.querySelectorAll('select').forEach((el) => {
const handler = this.fontFamilyHandler.bind(this, el.getAttribute('id'));
this.test.set(el, handler);
el.addEventListener('change', handler);
})
});
this.objectInstance.on('deselected', () => {
document.querySelectorAll('select').forEach((el) => {
if (!this.test.has(el)) continue;
el.removeEventListener('change', this.test.get(el));
})
});
(Note: using just an array would not be stable because if the number of <select> elements in the document changes, the indices would get out of sync.)
(Note: It would also work with Map instead of WeakMap, but the latter allows earlier garbage-collection of any <select> element that gets removed from the DOM.)
<select> which means the change event will only ever come from the <select> itself as it cannot have any children that would be able to send change events, using event.target.id works too:const eventHandler = event => this.fontFamilyHandler(event.target.id, event);
this.objectInstance.on('selected', () => {
document.querySelectorAll('select').forEach((el) => {
el.addEventListener('change', eventHandler);
})
});
this.objectInstance.on('deselected', () => {
document.querySelectorAll('select').forEach((el) => {
el.removeEventListener('change', eventHandler);
})
});
(Note: el.id is easier than el.getAttribute('id') and does the same - see Element#id.)
fontFamilyHandler directly and change it to read the ID from the element itself.this.objectInstance.on('selected', () => {
document.querySelectorAll('select').forEach((el) => {
el.addEventListener('change', this.fontFamilyHandler);
})
});
this.objectInstance.on('deselected', () => {
document.querySelectorAll('select').forEach((el) => {
el.removeEventListener('change', this.fontFamilyHandler);
})
});
// I removed the `key` argument here
this.fontFamilyHandler = evt => {
this.objectInstance.set(evt.target.id, evt.target.value);
this.canvas.requestRenderAll();
}
From the above comment ...
"One needs to see surrounding code in order to get an understanding of the actual
thisthe OP is dealing with ... I'm especially curious aboutfontFamilyHandler = (key, evt) => { /* ... */ }where this function floats freely versus line 3 where all of a sudden one seesthis.fontFamilyHandler.bind(this, el.getAttribute('id'));"
Until then the next provided answer assumes kind of a class syntax/system for a hypothetical MyType class.
Of cause the binding as with ...
this.fontFamilyHandler.bind(this, el.getAttribute('id'));
... is not necessary at all because the OP already does access the event.target within this handler which equals the above el ...
fontFamilyHandler = (key, evt) => {
this.objectInstance.set(key, evt.target.value);
this.canvas.requestRenderAll();
}
Thus, binding each element's id is not necessary since this property can be accessed via evt.target.id as much as the value already via evt.target.value.
In the end the code will boil down to something much more readable like ...
//class MyType {
// constructor() {
// this.objectInstance = { on: () => {} };
this.objectInstance.on('selected', () => document
.querySelectorAll('select')
.forEach(el =>
el.addEventListener('change', this.fontFamilyHandler)
)
);
this.objectInstance.on('deselected', () => document
.querySelectorAll('select')
.forEach(el =>
el.removeEventListener('change', this.fontFamilyHandler)
)
);
this.fontFamilyHandler = ({ target }) => {
this.objectInstance.set(target.id, target.value);
this.canvas.requestRenderAll();
};
// }
//}