Posted by lightsurge on April 5, 2012 at 2:55pm
3 followers
| Project: | Julio |
| Version: | 7.x-1.x-dev |
| Component: | Code |
| Category: | feature request |
| Priority: | normal |
| Assigned: | jgraham |
| Status: | needs review |
Issue Summary
At the moment the nivo slides can be linked to a url, which I think is good, however, it isn't clear a link is there, as there's no 'read more' link.
Also, as an alternative to supplying a url, it would be nice to be able to easily reference an existing node on the website.
For example, as a school I might want to promote an achievement on the slideshow, but to do so at the moment I would have to find the url to the node manually... and then julio doesn't make it clear there's any further information available.
Using node references instead we could also do the trimming of teaser text programatically, and hide all the more confusing field options in the slideshow node form, so all we have to fill in here is image and node reference.
Comments
#1
Related #1482884: Gallery and media related issues
#2
Unfortunately we don't have much control over this as we're using the Media Nivo Slider module. I'm moving this to the Media Nivo Slider queue, so feel free to follow up there.
#3
#4
Unfortunately, I don't feel the above recommendations are really in line with what I want for the contrib releases of this module. The module is mainly intended as a way to easily take Media Gallery content and provide it for display via the Nivo Slider plugin and let the plugin handle all the display aspects and how the content is presented based on the available plugin settings.
Making modifications to the presentation by adding extra indicators or behavior that isn't standard to the plugin I feel would be confusing and unexpected to users who have seen the standard plugin and expect this module to provide that same presentation. That being said there is absolutely no reason you can't take the module and modify it to better suit your needs and requirements.
In terms of changing or adding the link functionality to use node reference rather than URLs, while I agree that in some use cases it may be easier to link to a node if the field was a node reference, I'm against tying the link functionality so directly to nodes. The link field as well as the caption are intended to remain flexible in allowing you to link to any URL regardless of backing entity or location of content. The same holds true for the caption, while it would be convenient in some cases if the slide caption was populated by the node teaser, doing so would limit the ability to create captions independent of content without having extra fields.
#5
We may not necessarily need the media_gallery module to include this. Below snippet is a proof of concept to use core autocomplete to help a user find internal links. It's subject to #93854: Allow autocompletion requests to include slashes (unless the module uses its own autocomplete.js) which will fix a problem with forward slashes, but it looks like that's due for inclusion in drupal 7.13
Better still, now we have #1524920: deprecate nivo-slider caption field in favor of description, it's fairly easy to add a read more button to the caption, such that it's appended to the nivo slider caption on node save/update when a link is present but not into the editable description, so we're not confusing users by altering their input. Attached screenshot shows a read more button on a nivo slider from another project, a similar style of read more button might work here?
<?php
function julio_slideshow_form_alter(&$form, &$form_state, $form_id) {
if (strpos($form_id, 'media_edit_') === 0) {
if (!isset($form['#after_build'])) {
$form['#after_build'] = array();
}
$form['#after_build'][] = 'julio_slideshow_media_multiedit_form_after_build';
}
}
function julio_slideshow_media_multiedit_form_after_build($form, &$form_state) {
if ($form['media_nivo_slider_image_link']) {
$form['media_nivo_slider_image_link']['und'][0]['value']['#autocomplete_path'] = 'julio_slideshow/autocomplete';
}
return $form;
}
function julio_slideshow_autocomplete($string = ''){
$args = func_get_args();
$string = implode('/', $args);
$matches = array();
if ($string) {
$result = db_query_range("SELECT alias FROM {url_alias} WHERE (alias) LIKE :string", 0, 10, array(':string' => '%' . $string . '%'));
foreach ($result as $record) {
$matches[$record->alias] = check_plain($record->alias);
}
}
print drupal_json_output($matches);
}
?>
We could even feasibly check if the link is internal and pull a caption / title from a node if so... But I've changed my mind on that and actually think it's better to allow a custom one, because we do have very little space here and trimming would likely result in a non-sensical sentence.
#6
autocomplete works quite nice:
#7
actually, maybe doing this is better:
<?php$result = db_query_range("SELECT nid, title, status FROM {node} WHERE (title) LIKE :string AND status = 1", 0, 10, array(':string' => '%' . $string . '%'));
foreach ($result as $record) {
$path = drupal_get_path_alias('node/' . $record->nid);
$matches[$path] = check_plain($record->title . ' (' . $path . ')');
}
?>
#8
I have split the "Read More" functionality into #1528702: Caption overflow on excessive text in nivo-slider images let's keep this discussion focused on the autocomplete feature.
I like the idea of the auto-complete here as it improves the UX considerably.
#9
This patch adds:
#10
#11
Forgot the hook_menu, also, not sure why I used after_build here.
Setting this back to active as I'm sure the maintainers get the idea and I can't see it being committed (coding standards, the autocomplete being a bit convoluted etc.) even if fixes for the above added.
#12
Marking "needs work" as indicated by lightsurge's comments in 11.
Also, in general after_build is required when you want to modify fields as they just have stub entries in form_alter and haven't gone through their whole render chain yet.
#13
Patch now includes the relevant hook_menu
Also moved all the form alterations back into the form_alter, it didn't seem to make any difference in this scenario
#14
Wrong patch... here's the right one.
#15
The Drupal core fix that this patch relies on is present in the 7.14 release of yesterday