create icon indicating copy to clipboard operation
create copied to clipboard

Editing/Editor widgets's disable()/enable() shouldn't be called by Create, they should react to state changes

Open wimleers opened this issue 13 years ago • 4 comments

From IRC:

WimLeers: bergie: We're also having problems with the state changes not being specific enough: "In general, I think it should be the case that when you do $element.createEditable('setState', 'candidate'), that it should FIRST set the state (and trigger events) for the Editable widget and THEN propagate it to every predicate and also trigger events for those." bergie: it enables the editing widgets bergie: but indeed, we should mirror the new states for editing widgets instead of just using enable/disable WimLeers: also, .disable() is not being called on drupalformwidget anymore WimLeers: plus, we should be able to call disable() on going from any state back to candidate stage, NOT when going to inactive stage WimLeers: bergie: basically, this makes too many assumptions: _doSetState: function (previous, current, predicate) { this.options.state = current; if (current === 'inactive') { this.disable(); } else if ((previous === null || previous === 'inactive') && current !== 'inactive') { this.enable(); } bergie: disable mirrors the old disabled state of editable bergie: but agreed, if we add all the states as separate calls in editing widgets, then a widget has more control on what happens in each stage bergie: enabled/disabled was fine for rich text editing bergie: ...but for the other stuff, not necessarily bergie: can you make another ticket? WimLeers: k

wimleers avatar Nov 05 '12 15:11 wimleers

This means removing the old enable/disable methods and replacing them with per-state methods, so that there is more flexibility in controlling each editing widget.

One important thing is to ensure that the state changes are cancellable via the acceptStateChange callback before they propagate to editing widgets.

bergie avatar Nov 05 '12 15:11 bergie

This is the next (and I think last, besides #140) wall I'm running into.

The problem is not quite as simple as I had first outlined. The way it currently works, if a single property is in e.g. the changed state, then the entity is as well. This makes some sense. (i.e. "bottom up": if a single property's state is changed, then it should apply to the entity as well.) OTOH, if the entity is currently inactive and we transition it to candidate, then all properties should also inherit candidate, and in fact the state change should be triggered on each predicate specifically as well. (i.e. "top down": if the entity's state is changed, then all its properties should also receive this event.)

However, doing both "bottom up" and "top down" for all state changes doesn't make any sense in some cases (though it does seem to make sense in most). E.g. it should be possible to change a single property's state to inactive without also affecting the state of the entity. In any case, we shouldn't be making assumptions about how this should work. It can vary depending on the implementation, most likely.

It almost seems like we should track state of the EditableEntity widget and for each of its PropertyEditor widgets separately. And fire separate events (one at the entity level, the other at the property level). And in that case, it seems like it should be up to the app controller to control event bubbling (property -> entity) and event capturing (entity -> property), much like in JS?

wimleers avatar Nov 06 '12 23:11 wimleers

Running into this problem with Edit on Drupal7, see https://drupal.org/node/1893700

theodoreb avatar Jan 22 '13 10:01 theodoreb

Ok this is pretty critical to me, it's really messy to try and work around it.

theodoreb avatar Jan 23 '13 11:01 theodoreb