hook_image_effect_info() allows you to include a 'help' text with your effect definition.

"help": (optional) A brief description of the effect that will be shown when adding or configuring this image effect.

However, image_effect_form() never displays the help text.

Fix: Display the $effect['help'] information if present at the top of the effect add/edit form.

CommentFileSizeAuthor
#14 658068-theunraveler-effect_help_text.patch1.14 KBtheunraveler
#1 658068-eojthebrave-effect_help_text.patch643 byteseojthebrave

Comments

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Priority: Normal » Critical
Status: Active » Needs review
StatusFileSize
new 658068-eojthebrave-effect_help_text.patch643 bytes

This should do it. And marking as critical since this really should be fixed for Drupal 7.

matason’s picture

matason commented
Status: Needs review » Reviewed & tested by the community

This patch applies cleanly and displays the effect help at the top of the form.

Status: Reviewed & tested by the community » Needs work
Issue tags: -Novice

The last submitted patch failed testing.

Status: Needs work » Needs review
Issue tags: +Novice

matason requested that failed test be re-tested.

dries’s picture

dries commented 
+++ modules/image/image.admin.inc
@@ -378,6 +378,9 @@ function image_effect_form($form, &$form_state, $style, $effect) {
+  $form['effect_help'] = array(
+    '#markup' => isset($effect['help']) ? $effect['help'] : '',
+  );

Without studying the surrounding code, is there a reason why we don't use $form['#markup'] instead of $form['effect_help']['#markup']? (I should probably look at the surrounding code.)

matason’s picture

matason commented

Dries, I am not sure exactly why but $form['#markup'] doesn't seem to get picked up, it seems to need to be nested...

dries’s picture

dries commented
Priority: Critical » Normal

A missing help text is not a critical bug. It is an important bug, but not one that should block a release.

Status: Needs review » Needs work
Issue tags: -Novice

The last submitted patch, , failed testing.

Status: Needs work » Needs review
Issue tags: +Novice

Re-test of from comment #2386316 was requested by @user.

matason’s picture

matason commented

Dries, regarding #7, I completely agree, this is an easy fix, #1 does the job.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Is there a reason this isn't in hook_help()?

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented

I believe it was done this way so that modules which add additional effects could declare the help for those effects in hook_image_effect_info(). Though image.module could collect them into hook_help(), or every module that declares image effects could implement hook_help() itself. Preference?

theunraveler’s picture

theunraveler commented
StatusFileSize
new 658068-theunraveler-effect_help_text.patch1.14 KB

Here's a patch that displays the help text with hook_help().

eojthebrave’s picture

eojthebrave
he/him
Primary language English
Location Minneapolis, MN
commented
Status: Needs review » Reviewed & tested by the community

#14 is the right way to do this. Thanks!

Assuming this passes tests (and I can't see why it wouldn't) this is ready to go.

Maybe not a critical issue, but an important one for sure. Sticking with the theme of making Drupal 7 the most user friendly version of Drupal ever lets make sure this gets in. There are no API changes and it is a super simple change fixing something that should have been working in the first place.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Great. Thanks a lot, folks!

Committed to HEAD.

Status: Fixed » Closed (fixed)
Issue tags: -Novice

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

Issue tags: +Novice

Restoring issue tags, see #2125755: System messages removed all issue tags during D7 upgrade.