Fix setting/removing image in covers from /blog page #4682
Conversation
|
This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged. |
|
This PR targets the un-managed branch odoo-dev/odoo:master-mysterious-egg, it needs to be retargeted before it can be merged. |
| @@ -70,6 +71,8 @@ class CoverPropertiesOptionPlugin extends Plugin { | |||
| if (cancel) { | |||
| return; | |||
| } | |||
| editingElement.classList.add("o_dirty"); | |||
There was a problem hiding this comment.
It does not feel right to have to put it here. It seems that in master, it was handled by _setObserver in the wysiwyg adapter. There is probably something to do in handleMutations of the save_plugin instead 👍
There was a problem hiding this comment.
The issue here is that there is (sometimes) no mutations on the element but only on its descendants. But the (weird) way the information about the background is saved, requires the clean_for_save_handlers to be called on the element (not its child). For the handler to be called, it needs the o_dirty. How would you change save_plugin to handle this case? (I have no good idea)
There was a problem hiding this comment.
The issue here is that there is (sometimes) no mutations on the element but only on its descendants
Why is that? What it seems to happen in master is that some classes change on the editingElement -> o_dirty is added on the element as well. In this version, .o_record_has_cover seems to be added or removed from the editingElement when the background image changes which means that o_dirty should be added on the element as well. Maybe a selector is not correct in handleMutations and that is the reason why o_dirty is not added? To be investigate 👍
There was a problem hiding this comment.
There has been a recent change in the way mutations are generated about classes. Now it register which classes are added or removed, instead of registering everything written to it. So when o_record_has_cover is added, if it was already present, it does not create a mutation (so no o_dirty is added).
There are 2 situations where this is an issue:
- we already have a cover, and we change it to another one. Applying
classActionforo_record_has_coverdoes nothing because the class is already there - when we remove a cover from the
/blogpage. For some reason, there is not theo_record_has_coveron that page (this is where I noticed the issue, and also the issue that there may be several covers to be saved, hence the loops)
If there is an actual mutation (the class is added when it was not present), o_dirty is correctly added on the editingElement. If there is only mutation on the element that contains the image, o_dirty is added on that element, because it matches oeFieldSelector in the save plugin. Do you think this selector should be changed to not match this element (so o_dirty would end up on the parent)?
There was a problem hiding this comment.
Ok, I see, thanks for the clarification! Actually,another problem is that for example in edit mode you:
- Click on the blog post cover.
- Change the filter intensity.
- Save and come back in edit mode.
-> the filter intensity has not changed.
Those problems appear because, in our branch, we do saveToDataset at clean_for_save but because there were no clear modification on the editing element in itself, saveToDataset is not executed correctly.
There were no problem in master as _updateSavingDataset was done directly at action execution.
It feels to me that the cleanest way to do would be to do the same in our branch what do you think?
There was a problem hiding this comment.
I don't think that doing the saveToDataset during action execution would be very clean because we would not be able to use the classAction and styleAction as we are doing now and would need to re-implement them inside in a new one that does that + saveToDataset. (and also, we would have to copy the bit of logic to get the background because the save plugin needs its value after the before_save_handlers)
This is possible, but what do you think of adding a resource for get_dirty_els to add the covers that have a component that has changed?
dae8e15 to
777bc9a
Compare
No description provided.