nathanwoulfe / NestingContently

A property editor for disabling Nested Content items and Block elements in Umbraco
9 stars 8 forks source link

Property value is not updated for nested Nested Content #20

Closed ronaldbarendse closed 4 years ago

ronaldbarendse commented 4 years ago

Currently, NestingContently (version 2.1.2 on Umbraco 7.6.2) doesn't update the property value for Nested Content that's nested within Nested Content: the toggle only works on the first level.

The UI does make the nested item opaque, but the the property value of that element isn't updated, so saving doesn't store the correct value and reloading the UI shows the item as enabled again (the default state).

It might be good to add a note for this to the documentation (until a fix is available) 👍

We currently don't have very good use-cases for having NestingContently on nested NC, so we'll now only add the property/composition to first-level NC items. For some items (e.g. a section with nested NC for the columns) this might even be for the better, as most of the time they have a minimum number of items configured and disabling items would circumvent this minimum ;-)

greystate commented 4 years ago

I'm seeing similar behavior in Umbraco 8.6.3 even at the 1st level. Seems to have to toggle and save a couple of times for it to save the correct value.

Update: I can reliably reproduce it now. When the items are collapsed I can toggle them on and off, and it works. If an item is open and I toggle it on or off, the UI updates but somehow the value doesn't save.

nathanwoulfe commented 4 years ago

Pretty sure I know what's happening here - the disabled state is set on the nested item, but not on the instance of the nested item in its parent. Because nesting contently is a directive, there's probably some isolate state trickery going on, so changes don't propagate correctly. I think 🤷

nathanwoulfe commented 4 years ago

@ronaldbarendse @greystate I've spent a bit of time digging into this, with limited results...

NC does some funky stuff with cloning the model and re-assigning it and I can't work out how/where to hook into that process. I've tried passing all sorts of different scopes to try and find the actual property editor data to modify, and not having any luck. NC uses both a component and a directive, with one sending events into the other, both of which seem to be responsible for updating separate models. Seems super convoluted (in Plumber, for example, I need to trigger a click event on all open NC instances to re-bind the model when I auto-save content. It's pretty gross).

That means that if the NC panel is open, the nesting contently value isn't updated. If the panel is closed, everything works fine (on top level and nested NC instances). For now, to work around that, I'm going to hide the header button when the panel is open.

nathanwoulfe commented 4 years ago

Annnd after walking away, I've had what might be a good idea...

Rather than trying to update the property value in my directive, all it needs to do is emit an event describing the toggle state, which the nesting contently editor can consume and use to set its value - that should remove all the scope issues, because I'll just be setting the property value like any other custom editor, so NC's inner workings should be fine.

That will also make life easier when looking at block list support too...

greystate commented 4 years ago

I've just tried Nesting Contently III (alpha build 000071) and I can confirm that I'm able to consistently toggle items on and off, no matter whether they're expanded or not AND no matter whether they're items in NestedContent or the BlockList.

Hooray! 🔥 🌶️ 🌟 - well done @nathanwoulfe 👍

nathanwoulfe commented 4 years ago

@greystate glad to hear it, especially since I published V3 yesterday. 😁

Whole thing is a lot simpler now, manages to avoid all the syncing strangeness in Nested Content, which is a big win. 👍

ronaldbarendse commented 4 years ago

I can confirm this is fixed in version 3 👍