ymcatwincities / openy

The Open Y platform. See README.md below
https://openy.org
GNU General Public License v3.0
49 stars 111 forks source link

WYSIWYG - Entity Embed Image Link Doesn't Work #2184

Open dwells-tn opened 3 years ago

dwells-tn commented 3 years ago

Issue Description: The entity image embed link field doesn't work online.See https://sandbox-lily-ext-d9.openy.org/news/photo-link-test for a demo (picture should link to /locations).

Steps to recreate

podarok commented 3 years ago

Added Deprecation notice for the form See https://youtu.be/yYD44eTtv-E Functionality is already on Drupal 9 sandboxes image

podarok commented 3 years ago

@sarah-halby User Story

Given: I'm the content manager of Open Y instance and I need to add link wrapper over embedded image entity in WYSIWYG

Solution pre entity_embed upgrade: I can add Image by clicking the Image button in WYSIWYG and use the "Link to" field to provide a link over the image.

Solution after entity_embed upgrade: I can't use the "Link to" functionality because it is deprecated. If I try to use it - there will be an error message - "Link to is deprecated. Remove its value and use WYSIWYG Link button over embedded entity". I need to clear the "Link to" field to proceed Embedding entity. In order to add a link, I need to select an entity in the WYSIWYG content area and click the "Link" button in the WYSIWYG buttons panel to provide a link and link title.

Requirement: Old "Link to" empowered embedded entities will continue to work until Open Y removes the compatibility layer in 2021. After removal - Link to the field will disappear, link over embedded entity won't work.

Video for how it is working now https://youtu.be/yYD44eTtv-E

sarah-halby commented 3 years ago

@podarok feedback from @dwells-tn: It looks fixed. Can't test on Virtual Y, but it looked like it worked on News and Blog Post. One thing, though - I think if the field is deprecated, it needs to be hidden. I completely missed the helper text underneath the field, and I assume most folks will miss that. Once we address that, we can close this item.

podarok commented 3 years ago

@sarah-halby and @dwells-tn In order to deliver an upgrade path, we can't hide a field just now. Think about the amount of content we have on a field of associations to date. Due to providing automated upgrade path is almost impossible given how stretched our time is and how complex this task would be, we are relying on content managers to work with changing their content over time. Which means

Let me know if this makes sense and if we still need to hide this field now.

podarok commented 3 years ago

@sarah-halby this one is fixed. We can close it, I guess