A bit of work went into D7 version of edit module. since it relies on an attirbute and a specific set of classes and HTML structure using DS could make all of this break.

What edit module is using for fields right now is as follows:

  1. A specific permission can be checked for users able to use inline edit.
  2. hook_preprocess_field() to add a data-edit-id attribute.
  3. field_view_field() to rerender the field with the new value the user just changed.
  4. The trick is that the current CreateJS/VIE implementation relies on this HTML structure to work:
    <span data-edit-id="entity_type/entity_id/field_name/language/view_mode" class="bunch of classes">
  <span class="field-item">
    field value, html or whatever.
  </span>
</span>

    The wrapping .field-item is still unfortunately required for now. It could be changed by tweaking the EditService.js file but that's a bit complicated/messy.

I think that's all that might confilct with DS.

CommentFileSizeAuthor
#4 1896550-ds-4.patch3.04 KBswentel
#3 1896550-ds-3.patch2.04 KBswentel
#2 1896550-edit-2.patch549 bytesswentel
#2 1896550-ds-2.patch552 bytesswentel

Comments

Anonymous’s picture

Anonymous (not verified) commented

Microdata also requires attributes on the field item wrapper for string and entity reference type fields.

swentel’s picture

swentel commented
Status: Active » Needs review
StatusFileSize
new 1896550-ds-2.patch552 bytes
new 1896550-edit-2.patch549 bytes

So, the most breaking part is that DS also has its custom fields, which are themed through theme_field(), but are not actually Field API Fields, this breaks edit completely.

A quick fix *could* be this, cf patches attached, also required for edit. This way, it doesn't break anymore and you don't get the 'The requested page could not be found.' popup anymore. Field API fields work with this, haven't checked with the pseudo fields yet, but that requires some changes in DS anyway.

Second problem is that it is possible to select different field templates. By default it's using theme_field(), but DS adds the ability to change these, sometimes using no wappers at all. I haven't tested with that yet. I guess that part can only be fixed in DS itself with a module_exists check.

swentel’s picture

swentel commented
StatusFileSize
new 1896550-ds-3.patch2.04 KB

Proof of concept to make the title pseudo field work as well if it's rendered as a DS field.

swentel’s picture

swentel commented
StatusFileSize
new 1896550-ds-4.patch3.04 KB

support for title, author and created date.

swentel’s picture

swentel commented

So the ds_edit_support() function (which could probably have better name, not sure), should also check the 'Access inline editing' permission I guess.

nod_’s picture

nod_
Primary language French
Location Lille
commented

umm we're outputing the markup even for users who don't have the permission in the edit module, we should probably use that too.

nod_’s picture

nod_
Primary language French
Location Lille
commented

Committed the edit part of this.

swentel’s picture

swentel commented
Status: Needs review » Fixed

Committed to DS 2 branch, will leave first branch as is for now.

Also, do you think we could create a patch for the '#skip_edit' property, although I can think that Wim won't like this one, I'll ping him in IRC for it :)

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Why would that property be necessary? :)

swentel’s picture

swentel commented

Same as I had in testing DS in D8 - My Display Suite fields - which are not Field API fields - are themed through theme_field() so you get consistent markup. At that moment the attribute is also added, but edit chokes on it because it simply has no idea what this field is.

It's a quick stopgap fix in a way, but I had no better idea for this :)

Anonymous’s picture

Anonymous (not verified) commented

As mentioned above, it isn't just Edit module that depends on hook_preprocess_field to place field-specific attributes. RDFa and microdata also rely on this. I know this issue is for D7, but just want to say that I hope we are not considering an Edit-specific stopgap for D8.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

My Display Suite fields - which are not Field API fields - are themed through theme_field() so you get consistent markup

IMO this is a bug.

You should introduce theme_ds_pseudofield() or something similar, because you're implicitly breaking an API here: you're invalidating the assumptions developers can make about theme_field() and thus consequently hook_preprocess_field().

This would also address @linclark's concerns, I think.

swentel’s picture

swentel commented

That would mean I have to rewrite everything and that's not an option for the D7 version :)

Yeah, maybe using theme_field wasn't the smartest option, maybe I should have just called theme_field() directly in my own theme function so the preprocess functions are skipped. I'll think about that for D8.

- edit - meh, directly calling theme_field is a problem as well as that's not reliable.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.