I created a page content type and added an unlimited value paragraph field to it. That paragraph field can add "Media" paragraph bundles. The "Media" paragraph bundle contains an unlimited value atom reference field.

When I add an atom to the paragraph's atom reference field, the ctools dropdown links no longer contain the edit and view links and the Representation Context select input is removed.

Here is an image that illustrates this:

Just wanted to let you know. Sometimes you see this happen when creating a node and sometimes you only see it happen when editing a node.

CommentFileSizeAuthor
#19 preview-should-not-be-rendering-context-atom-ref-2468431-19.patch4.17 KBnagy.balint
#12 preview-should-not-be-rendering-context-atom-ref-2468431-12.patch665 bytesnagy.balint
#7 preview-should-not-be-rendering-context-atom-ref-2468431-7.patch691 bytesnagy.balint
Edit test test | scald.jpg128.54 KBgmclelland

Comments

gmclelland’s picture

gmclelland commented

I'm going to try and reproduce on simplytest.me

gmclelland’s picture

gmclelland commented
Title: atom reference ctools dropdown links breaks when used with the paragraphs module » multivalued atom reference links break

Actually this doesn't have anything to do with the paragraphs module.

Steps to reproduce the problem:
- goto http://simplytest.me/project/scald_galaxy/7.x-1.x
- goto /admin/structure/types/manage/article/fields/field_image
- change the "Rendering context" to "Preview Representation" and change the Number of Values to "Unlimited"
- goto /node/add/article
- open the scald drawer and add three image atoms
- add one of the image atoms to the "Image" atom reference field on the article node
- Notice the "Operation" link in the upper right hand of the webpage?

I'm not sure what the best way to title this issue?

nagy.balint’s picture

nagy.balint commented

Hi!

So far i cant reproduce the problem.

nagy.balint’s picture

nagy.balint commented

Okey i got it.

The issue is only related to the preview representation, if you set that rendering context to anything else it works fine.

Generally the preview representation is used only for the qtip preview in dnd, not exactly sure if it was intended to be used anywhere else .

nagy.balint’s picture

nagy.balint commented
Title: multivalued atom reference links break » Preview representation as rendering context in atom reference breaks the widget.
nagy.balint’s picture

nagy.balint commented
Status: Active » Closed (won't fix)

The issue is that, the preview representation is not parseable, and it cannot be set parseable on the UI, so i think its safe to say that it was not inteded to be set up as rendering context in that area.

So the only thing we can do is to remove that representation from the list, as we already remove the library representation.

But of course you can make your own representation that can be made parseable and displays all the information you need. If you need help with that let me know.

nagy.balint’s picture

nagy.balint commented
Status: Closed (won't fix) » Needs review
StatusFileSize
new preview-should-not-be-rendering-context-atom-ref-2468431-7.patch691 bytes

The patch to remove the preview repr. from the list.

Status: Needs review » Needs work

The last submitted patch, 7: preview-should-not-be-rendering-context-atom-ref-2468431-7.patch, failed testing.

Status: Needs work » Needs review

nagy.balint queued 7: preview-should-not-be-rendering-context-atom-ref-2468431-7.patch for re-testing.

gmclelland’s picture

gmclelland commented

Good thinking @nagy.balint. To be honest, I still don't quite understand the difference between "Scald context" and "Rendering context"?

I also notice the behavior with the "Rendering context" set to "Title". We should probably hide that as well, right?

Scald Galaxy includes scald 7.x-1.x-dev

gmclelland’s picture

gmclelland commented

Tested the patch and it does remove the "Preview Representation" from the "Rendering context"

nagy.balint’s picture

nagy.balint commented
StatusFileSize
new preview-should-not-be-rendering-context-atom-ref-2468431-12.patch665 bytes

There is no real difference. Its just Atom reference fields do not support non parseable contexts (mostly the internal contexts like library and preview). You can find their definitions at scald_dnd_library_scald_contexts().
And the rest of the system contexts are defined at _scald_system_contexts()

And indeed, you are right, that it would be a better check if we checked whether a context is parseable or not, instead of picking a few.

gmclelland’s picture

gmclelland commented

I tested the patch in #12 and it does remove the parseable contexts.

Can you further explain what the difference between "Scald context" and "Rendering context" does?

I see that the "Rendering context" changes how the atoms are displayed on the atom reference field when editing the node, but what does the "Scald context" do?

gifad’s picture

gifad commented

Should not the check be updated in atom_reference_field_widget_form() too ?
But please don't filter out the title context in manage/display (I'm using it in teaser view mode)

nagy.balint’s picture

nagy.balint commented

Wow, you might have uncovered a merge issue ...

nagy.balint’s picture

nagy.balint commented

@gifad in manage display thats the title formatter, so that wont be harmed :)

nagy.balint’s picture

nagy.balint commented

I believe we have one too many context selections.

At the moment as I see the scald context widget setting is used to populate
$element['#preview_context'] = $preview_context;
But thats it.
It was to be used on display as well, but then we commited the patch to add view and edit links to the atom reference.
And unfortunately that patch overrode the display, and added another context selection on the instance level.
And so now everything works with this field instance level context, but the widget level context introduced by @yched is still there.

So maybe the easiest is to delete the widget based context selector.

Or we can remove the instance level one and rework the other code to always use widget settings.

What do you think?

nagy.balint’s picture

nagy.balint commented

It seems to me that it makes more sense as a widget setting, as basically that context is only to define the render on the widget itself, and has nothing to do with any other display. Going to make the adjustments like that.

nagy.balint’s picture

nagy.balint commented
StatusFileSize
new preview-should-not-be-rendering-context-atom-ref-2468431-19.patch4.17 KB

Attaching the patch that does that.

Since this new rendering context was only added after the last stable (1.3), and in 1.3 there is only this widget setting, and i kept the name of the original widget setting, and we kept the original widget setting even if someone upgraded to the dev, likely we dont have to do any migration here.

gmclelland’s picture

gmclelland commented

Sounds like a good idea. Patch seems to work for me.

gifad’s picture

gifad commented

Works for me too...

To answer my question at #14

Should not the check be updated in atom_reference_field_widget_form() too ?

the select box here is rebuilt in atom_reference.js from Drupal.settings.dnd.contexts[], built (and filtered) in dnd.module

nagy.balint’s picture

nagy.balint commented

Yes but that is different, because that is the context override for the display of the atom reference. While this patch only changes the context setting for the field widget on the edit form. And the two are different.

Thats why the formatter wont be modified here either, cause the formatter is for the display, while this patch here is for the form widget only.

  • nagy.balint committed 01f53cf on 7.x-1.x 
    Issue #2468431 by nagy.balint: Removing unnecessary atom reference...
nagy.balint’s picture

nagy.balint commented
Status: Needs review » Fixed

Thanks for finding this issue!

Fix committed.

Status: Fixed » Closed (fixed)

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