Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
Users would benefit from the ability to link an image to an arbitrary URL.
Possible use cases:
- A page with a grid of logos linked to sponsor pages.
- A rudimentary display advertisement.
Suggested implementation: an optional link field to the pane, and connect that with linkit.
Comment | File | Size | Author |
---|---|---|---|
#31 | panopoly8_widgets_image_link-2013965-3.patch | 5.92 KB | mfrausto |
#29 | panopoly8_widgets_image_link-2013965-2.patch | 6.69 KB | mfrausto |
#27 | panopoly8_widgets_image_link-2013965-1.patch | 6.69 KB | mfrausto |
#20 | interdiff.txt | 444 bytes | dsnopek |
#20 | panopoly_widgets-image-link-2013965-20.patch | 13.08 KB | dsnopek |
Comments
Comment #1
caschbre CreditAttribution: caschbre commentedWe could potentially just use a text or link field and then theme it so the image renders with the link. Using linkit would be ideal but requires an upgrade.
Related: #2186541: Upgrade linkit module 3.1+
Comment #2
lsolesen CreditAttribution: lsolesen commentedTagged #2271411: Add option to link images added via Add Image as a duplicate.
Comment #3
dsnopekI agree this would be a useful feature! I had a client ask for it, but it wasn't high enough priority to get implemented as part of that project. :-/
Comment #4
caschbre CreditAttribution: caschbre commentedNote: a patch needs review at #2186541: Upgrade linkit module 3.1+ that can open up the way to get this issue resolved.
Comment #5
dsnopekAttached is the start of a patch for this! It includes a linkit profile for use with fields, but tries to keep the dependency on linkit optional. I'm not entirely sure that panopoly_widgets is the best place for this profile, but I haven't had an enough time to think it through.
In any case, this needs:
Comment #6
dsnopekHere is a new patch - there are some minor changes to functionality and some tests. I've been having some trouble running the tests locally so I'm going to try then on Travis-CI in a moment.
Comment #7
dsnopekDidn't get the tests quite right - here's another attempt.
Comment #8
dsnopekCloser! Here's another attempt.
Comment #9
dsnopekHopefully, the final attempt!
Comment #11
dsnopekOk! The test is finally working. :-) Committed!
Comment #12
dsnopekSome upgrade tests are failing: #2382495: Image widget with Linkit tests fail after some upgrades
Comment #14
dsnopekDue to the issues with the upgrade tests, I've decided to actually revert this and fix it before committing again. I really don't want to leave the tests broken for a prolonged period of time.
That means I'm also closing #2382495: Image widget with Linkit tests fail after some upgrades as a duplicate.
And here is the description from it:
Comment #15
dsnopekHere is a new patch that attempts to fix the upgrade issues!
Comment #16
dsnopekHere's a new set of patches that fix the upgrade tests! I also make some minor tweaks to texts and the order of the fields.
However, visual layout of the "Link" field is kind of insane and I'd like to rearrange it before committing again:
I'd prefer to see:
Update: Here's the current Travis-CI run with these patches: https://travis-ci.org/dsnopek/panopoly/builds/42233548
Comment #19
dsnopekHere's a re-roll for the latest version of Panopoly and it removes some options/features which I think addresses my UX concerns in #16. If this passes tests, I'm going to commit it again - it's long overdue. :-)
EDIT: Here's the Travis build: https://travis-ci.org/panopoly/panopoly/builds/146656971
EDIT-2: That one was broken by an unrelated issue, here's a new one: https://travis-ci.org/panopoly/panopoly/builds/146666491
Comment #20
dsnopekTests failed because the Feature was overridden. Here's an attempt to fix that!
EDIT: Here's the Travis build: https://travis-ci.org/panopoly/panopoly/builds/146708202
Comment #22
dsnopekTests passed, so committed!
Here's a couple of follow-up issues that depended on this issue:
Comment #23
dsnopekComment #25
dsnopekThis feature needs to be ported to Drupal 8! I think this got lost in the shuffle because the Drupal 8 version of the Image widget was created before this was committed, then this was committed, and now Drupal 8 doesn't have this feature
Comment #26
dsnopekComment #27
mfrausto CreditAttribution: mfrausto commentedHere my first attempt to port this feature to panopoly 8
Comment #28
dsnopekThans!
Hm. I don't like that this is rendering in a preprocess hook. In D8, it's best to render as late as possible, ideally, just creating render arrays in PHP code, and only rendering in templates.
What type of element is used for 'field_panopoly_widget_image'? Perhaps it allows for a way to add a link?
Comment #29
mfrausto CreditAttribution: mfrausto commented@dsnopek Sure, Changed a little bit the way is rendering using render arrays...
I see 'field_panopoly_widget_image' is image type, Not sure if it allows to add a link, I'll research and try
Comment #30
dsnopekThe latest patch is better, but still not ideal. If there was a way to just put the URL on part of the existing render array in such a way that the image would get rendered wrapped in a link, that'd be the best outcome. But I guess that's what you're researching!
I don't know if these parts of the patch will remain after your research, but here's a couple quick comments:
This constant should instead be 'PANOPOLY_WIDGETS_IMAGE_BUNDLE' to namespace it for this module
This function should be prefixed with 'panopoly_widgets_' to namespace it to this module. Given that it's an internal utility function, I might put a leading underscore on there too. So, like '_panopoly_widgets_image_get_url_field()' or something like that
Comment #31
mfrausto CreditAttribution: mfrausto commentedThis is my attempt to improve the patch, also added update hook.
Comment #32
mfrausto CreditAttribution: mfrausto commentedComment #34
dsnopekThanks! Worked in my testing. :-) Committed with two minor changes: