Right now, when creating a DnD-enabled text field instance, you have to pick one context from the list of public context, with the default value being set to the default global context. This is all good, until the point site contributors ask the site builder to change the default context on every standard content.

At that point, you're now facing the not all that pleasant task of having to go through all of your text field instances, updating them to use the new default context, and if you're not a Scald developer and don't know about the hidden 'dnd_context_default' variable, you also have to take care of setting that context for newly created instances.

I think that whole process can be fluidified quite simply:

- Add a new option on the field creation form, '- Use the global default context -' and select that by default
- Remove the various call in mee.module that tries hard to fetch dnd_context_default, because there's already code to do that in the ckeditor plugin anyway
- Expose the dnd_context_default variable in admin/config/content/dnd

There should be no upgrade path issue:
- For sites upgrading, there will be no change by default ; site builders can change there fields to use the new context value
- Every time an atom is dropped into an RTE, the default context gets resolved, and then stored (either in the SAS or as an attribute of the div). Changing the default global context thus won't provoke unexpected change to user content, and only affect new drops. I think that's the sane thing to do, ensuring we don't garble user content.

Thoughts welcome ; I'll probably post a patch tomorrow.

Comments

DeFr created an issue. See original summary.

DeFr’s picture

Status: Active » Needs review
StatusFileSize
new4.29 KB

Here's what it would look like.

gifad’s picture

I'm not sure that the legacy scald/modules/fields/mee/plugins/ckeditor/plugin.js will make a correct use of an empty value for editor.element.$.attributes['data-dnd-context'].
Just looking at the code, it's not obvious...

gifad’s picture

This seems to handle every settings pattern...

gifad’s picture

StatusFileSize
new2.96 KB

Finally, I prefer giving back a little bit of simplification of the mee.module, but keep the whole client side unaffected...

DeFr’s picture

The old plugin version never respected the field setting with regard to what context to use when dropping, and always use the markup placed in the ['editor'] key for the library, so that's not a concern.

Patch in #5 can't work. The new default value would be '', which is empty, so the !empty() check won't work.

Patch in #4 is interesting though, because there's definitely no reason to handle the attribute being missing or empty in dndck4.js and not in plugin.js

DeFr’s picture

Taking a step back here, patch in #5 does work for the purpose described in this issue, so sorry for the previous comment @gifad. The attribute will be filled with the default context on form generation, which will be the same as the one set by DnD in the JS settings.

What I should have mentionned in the original issue summary is that this is the first part of a patch serie I'm working on, whose goal is to add a few options on the client side, through a few optional additional CKEditor plugins ; one of those tries to detect drops to atoms in RTE fields that are set to '- Use the global default context -' instead of being set to the context that happens to be the current default, and react accordingly. With the patch in #5, that falls apart, because it makes the two situation appears to be exactly the same to client side code.

gifad’s picture

To make your plugin "react accordingly" to this new feature, you can add in mee_field_widget_form_alter() something like $element['#attributes']['data-dnd-specified-context'][] or whatever...

DeFr’s picture

@gifad Feels weird one attribute saying "render in this specific context" ( data-dnd-context ), and another saying specifying "Ah, hum, wait, maybe the context isn't all that defined after all" ;-)

gifad’s picture

Well, to be more explicit, my proposed new attribute could be named ['data-dnd-uses-default-context'], boolean, to match precisely your new feature.
My point is to prevent existing code from "falls apart"...