Previously the link atom property was added to all atoms, but it was disabled on everything except images.

Of course there was a text explaining that the property is only available for the image provider. But it would be a lot nicer if it didnt even show up on other providers.

CommentFileSizeAuthor
#2 scald-settings-config-2428929-2.patch1.54 KBgifad
#1 hide-link-property-in-other-types-2428929-1.patch1.69 KBnagy.balint

Comments

nagy.balint’s picture

nagy.balint commented
Status: Active » Needs review
StatusFileSize
new hide-link-property-in-other-types-2428929-1.patch1.69 KB

Here is a patch that achieves that.

(There could be a follow up issue to solve the pluggability of these atom properties. As currently there is even a problem with backward compatibility. Basically if a provider wants to add a new atom property, it has to be written for both the old way of embed and the new. Which is unnecessary duplication.)

gifad’s picture

gifad commented
StatusFileSize
new scald-settings-config-2428929-2.patch1.54 KB

Hi nagy.balint,
ok for your patch;
now about pluggability of these atom properties :
1 - the name of "Atom properties" is wrong : there are "Atom embedding options"
2 - those options do not come from nowhere : they are implemented in either an atom provider, or a scald player, or a context provider;
3 - so the show/hide status of each option can be set if we know the rendering path that the atom will follow in scald_render();

the attached little patch sets a Drupal.settings.dnd.contexts_config to make this tracking possible (if not easy)
(this is the raw output of scald_context_config_load($context))
additionally, it adds Drupal.dnd.Atoms[id].meta.provider

NB: this makes sense only in the "new" (embed_div) storage method, where the "options" field can travel transparently all the rendering steps

jcisio’s picture

jcisio commented
Status: Needs review » Needs work

The first patch is ok to me, but the second is not complete: is there anything to do on the JS side? Could we merge into a single patch?

  • jcisio committed 47df343 on 7.x-1.x authored by gifad 
    Issue #2428929 by nagy.balint, gifad: To hide the link atomproperty...
jcisio’s picture

jcisio commented
Status: Needs work » Fixed

I think again and decide to commit #1. About #2, it will be a separate issue: to show the link option for other providers that support it.

gifad’s picture

gifad commented

but the second is not complete

About #2, it will be a separate issue

This is Pluggable Options for Atom Properties

Status: Fixed » Closed (fixed)

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