Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
We have several use cases of altering a field formatter settings and there is no easy way to do it. Patch adds two small alter hooks, hook_field_formatter_settings_form_alter() and hook_field_formatter_settings_summary_alter() since the latter is required in order to alter in settings to a formatter that does not natively provide any settings.
Comment | File | Size | Author |
---|---|---|---|
#55 | 945524-55-tests_only.patch | 2.93 KB | jthorson |
#55 | 945524-55.patch | 6.98 KB | jthorson |
#44 | drupal-field_formatter_settings-945524-44.patch | 3.23 KB | yannickoo |
#41 | field_formatter_settings_d7-945524-1.patch | 3.24 KB | jaffaralia |
#36 | 945524-36.patch | 7.25 KB | swentel |
Comments
Comment #1
Dave ReidInitial patch that requires the hook docs to be fleshed out, but works.
Comment #2
mfer CreditAttribution: mfer commentedTo make module like Field Formatter Class even reasonable for mere mortals this patch is really needed.
I tested and the code works. Looking forward to the docs.
Comment #3
webchickCan you make the case that this is a regression from CCK in Drupal 6?
Comment #4
mfer CreditAttribution: mfer commentedThis is sort of a regression and sort of a new feature that has been largely unused by many.
In CCK Formatters did not settings. So, they did a few things to overcome this. One example is the way imagecache worked with imagefield for the display settings. Since you couldn't specify settings all the available settings were listed for each imagecache preset. This list could be altered fairly easily. Another example is the swftools module that created a global config and housed it on custom config pages. These could be easily form altered and were straight forward forms.
But, the way D7 does it is totally new. Formatter settings are not bolted on through something else and this was a late addition to D7. So, we went from CCK with a bolt on formatter setting solution you could easily alter (even if just at the form level in fairly straight forward forms) to native formatter settings you have to be a FAPI guru with a lot of patience to begin to hack on (the formatter settings forms are not trivial to work on).
Because this is done in a whole new way from D6 CCK and we are just now finding in real usage there are some problems I like this patch. It does not change the API but does add two new alter calls (which are only called on the formatter settings page). I've already seen two separate use cases for this functionality. One I linked to.
Hope this helps.
Comment #5
yched CreditAttribution: yched commentedEven if you can alter the settings form and summary, how do you reflect the actual changes in the rendered array ?
We intentionally did not include an alter hook on 'formatted field output' render arrays, because such a hook would be called potentially 100s of times on listing pages. So right now the only way is hook_field_attach_view_alter(), which operates on the render array of the whole entity - you thus have to find your way in there to the sub array(s) you want to alter.
Note that widgets settings forms are currently not alterable either. If one is, the other should too for consistency.
Comment #6
mfer CreditAttribution: mfer commented@yched - If you want to add or alter the structure I see no other way than to use hook_field_attach_view_alter() and I think that is acceptable for that. But, there are other cases where you may want to alter the config form in a way that has no impact on altering the render array. For example, what if someone wants to limit the image styles listed to a subset of the total image styles. This could be done through the settings form without altering the rendered output.
Comment #7
andypost+1 from #961412: Formatter settings button lost when summary is empty
EDIT
comming from #553292-13: Formatter settings lost when saving a field
and #830704-29: Entity forms cannot be properly extended
Comment #8
RobLoachThe image formatter field system is so gross... theme_image_formatter, theme_image, theme_image_style, it's all so gross and ugly.
Comment #9
larowlansubscribe (sorry)
Comment #10
amateescu CreditAttribution: amateescu commentedsubscribe
Comment #11
andrewmacpherson CreditAttribution: andrewmacpherson commented+1 for this. Now we're into D8, can we look into this API addition some more?
The Field formatter settings API module implements this in D7 contrib.
So far there are 4 contrib modules using these new hooks, which demonstrate their usefulness well.
Comment #12
andrewmacpherson CreditAttribution: andrewmacpherson commented#1: 945524-field-formatter-settings-alters-D7.patch queued for re-testing.
Comment #14
tim.plunkettRerolled with git prefixes, and fixed the spelling of summary.
Still needs the api.php docs to be created.
Comment #15
RobLoachUmmm, I made #1308860: Allow modules to alter fields settings hook, and either completely forgot about this issue, or was thinking of something else at the time. I'm going to mark that one as a duplicate. Either way, here's what I wrote up there...
Comment #16
tim.plunkettRerolling, also moving $form_state out of the $context into a proper variable, like hook_field_widget_form_alter has it.
Comment #17
pfrenssenI fixed a typo ("formater") in the patch from #16 and added a comment to the second call to drupal_alter().
Comment #18
andypostpatch is good but no go without tests :(
Do we have a same issue for field settings? EDIT reopened #1308860: Allow modules to alter fields settings hook
Comment #19
tim.plunkettCNW for tests.
Comment #20
Jelle_SWritten tests to see if the new alter hooks work. First patch: tests only, second patch: patch from #17 with added tests.
Comment #21
Jelle_STrailing whitespaces removed...
Comment #22
Jelle_SOk, now I know the test pass:
Punctuation... (the devil is in the details)
Comment #23
Jelle_S#22: drupal-945524-22.patch queued for re-testing.
Comment #25
Jelle_SReroll...
Comment #26
swentel CreditAttribution: swentel commentedLooks good and still applies, let's do this!
Comment #27
andypostContext should be described in php doc for hook_field_formatter_settings_form_alter()
Context should be described in php doc for hook_field_formatter_settings_summary_alter()
Comment #28
Jelle_SAdded phpdoc.
Comment #29
attiks CreditAttribution: attiks commentedThis looks good to me.
Comment #30
webchickWe're over thresholds atm so I unfortunately can't commit any feature patches. Help with smashing critical and major bugs would be greatly appreciated.
Comment #31
yannickooWe are over, since when?
Comment #32
xjm@yannickoo: You can see the number of major and critical bugs in the contributor links dashboard blog. The thresholds are 100 each major tasks and bugs; and 15 each critical tasks and bugs. See the threshold policy for details.
The test coverage looks good. However, I think we need to add example implementations for the hook documentation rather than empty stubs.
NW for that change.
Also, some minor code style issues (see below).
These comments are over 80 characters and need to be rewrapped. Also, hook and function names should always have
()
in inline documentation.I'll fix what I can here.
Comment #33
xjmAttached:
t()
from the custom assertion message (see http://drupal.org/simpletest-tutorial-drupal7#t).NR for the sample implementations. Please check over them; I didn't actually test them.
Comment #34
xjmComment #36
swentel CreditAttribution: swentel commentedComment #38
swentel CreditAttribution: swentel commented#36: 945524-36.patch queued for re-testing.
Comment #39
Stalski CreditAttribution: Stalski commentedPatch is good, ready to be committed imo
Comment #40
webchickThis looks like a nice simple addition and it comes with test coverage. Thanks!
Committed and pushed to 8.x. I believe this should be eligible for backport, so marking as such.
Comment #41
jaffaralia CreditAttribution: jaffaralia commentedHi,
I created patch for D7.
Thanks,
A. Jaffar ali
Comment #42
yannickooWhen the patch for drupal 7 wil reviewed, when could it be in core? Will there be any conflicts with field formatter settings module?
Comment #43
attiks CreditAttribution: attiks commented@jaffaralia: Thanks, but can you also backport the tests?
spaces
@yannickoo: you can review/test this yourself
Comment #44
yannickooHere is the patch without the whitespace. I don't know how to customize the test because I don't know how to write them yet :/
Comment #45
yannickooBTW there is a typo in the test for 8.x:
It should be hook_field_formatter_settings_form_alter. The "f" from field is missing.
Comment #46
attiks CreditAttribution: attiks commentedCan you create a new issue with a patch to fix it?
Comment #47
yannickooOh, I think webchick fixed the typo in her commit.
Comment #48
yannickooIt's very difficult to backport the test because in drupal 7 there is no test module, just the field_ui.test and I don't know how to implement the hooks in the test file. Please advise how to do this.
Comment #49
attiks CreditAttribution: attiks commentedYou have to create a test module for this, but I think easy way is to copy/paste it from Drupal 8, get rid of all the code that isn't needed and create a new SimpleTest that enables this module.
Comment #50
swentel CreditAttribution: swentel commentedOk, moving back to 8.x first as this simply doesn't work - without a work around though. Found this out while working on #1234624: Limit the number of fields to display where I started implementing this hook in field ui as that makes sense.
The key problem is in field_ui.admin.inc line 1379
Now, there is a work around :
I can live with that, but then we should make sure we update the documentation in field.api.php. Thoughts ?
Comment #51
swentel CreditAttribution: swentel commentedOk, investigated how the modules in 7.x do this and they actually implement hook_field_formatter_info_alter() which makes sense, let's make sure we document this in the api.php so people know what todo.
Comment #52
swentel CreditAttribution: swentel commentedMoving back to 7.x - I also though the tests was bogus, but it isn't, let's try to get this ok for 7.x first.
Comment #53
Fabianx CreditAttribution: Fabianx commented#44: drupal-field_formatter_settings-945524-44.patch queued for re-testing.
Comment #54
Fabianx CreditAttribution: Fabianx commentedComment #55
jthorson CreditAttribution: jthorson commentedD7 backport, including the D8 tests.
Comment #56
Fabianx CreditAttribution: Fabianx commentedThat is a straightforward backport. Looks good, applies and passes tests => RTBC
Comment #57
jthorson CreditAttribution: jthorson commentedAnd for bonus points, we're going to want this for D7 project_issue, which in turn impacts the D7 port of Drupal.org. ;)
Comment #58
yannickooMeans that this feature could be in the next Drupal 7 release and all the modules which depend on Field formatter settings module won't need that module as dependency.
Comment #59
jthorson CreditAttribution: jthorson commentedUh-oh ...
... Do we have a naming conflict between this patch and Field formatter settings? Given that the hook is named the same between the two, will anyone with Field formatter settings applied end up double alter()-ing their forms if they upgrade to a core release using this?
Comment #60
yannickooIn the hook_update_n hook we have to disable and uninstall the other module right?
Comment #61
Fabianx CreditAttribution: Fabianx commented#60: We cannot disable a contrib module from core, but we can open up an issue on the contrib module to deprecate the module when 7.xx ships. Given that Dave Reid is the maintainer, I guess that will be no problem.
Comment #62
yannickooOh sorry, absolutely not in the core but in the contrib modules which depend on that module. It's important to notify the maintainers from the modules which require it and Dave to close his (great) project.
Comment #63
klonosI guess that if Dave wants to take it easy on the ~3000 users of the module, he could release a 7.x-1.1 or 7.x-2.0 version that checks core version before it does its magic:
- If installed core version < 7.xx, then work the same as 7.x-1.0.
- If installed core version >= 7.xx, then only display a warning/message of some kind (site status page perhaps?) to disable and uninstall the module.
Does that make sense?
Comment #64
yannickooSounds good but I wouldn't put that message on the status page. I would check that in
hook_init
and display a message viadrupal_set_message
with a warning level.Comment #65
swentel CreditAttribution: swentel commentedThere's an easier approach and that's simply won't fixing this issue for D7. The module exists and is stable. I guess it's Dave's call.
Comment #66
Fabianx CreditAttribution: Fabianx commented#65: Please see #57. It would make the life of D.org D7 easier without adding yet another module dependency ...
Comment #67
yannickooNot only for D.org D7 it is better because you don't have to install an extra module for this little functionality.
Comment #68
swentel CreditAttribution: swentel commentedOk, missed #57, it's just that it's sometimes that people will miss these kind of announcements, I'm ok with both :)
Comment #69
yannickooTo avoid misunderstanding this means that we just have to wait for a D7 administrator who will commit #55?
Comment #70
David_Rothstein CreditAttribution: David_Rothstein commentedSeems like it would be a good idea to get some feedback from Dave Reid here, since he is the maintainer of the Field formatter settings module.
Looking at that module's code, I'm also noticing that it does some things with Views, so I'm not sure it would actually be as simple as turning off that module if/when these hooks get added to core.
To be honest, if that module didn't exist, we'd probably just add the hooks to core. But it does exist, has 3,000 happy users, lots of other modules that depend on it, etc. Maybe this is a case where we should just leave well enough alone? :)
Comment #71
yannickooOkay, so in Drupal 7 there is no way to get this feature in. I can understand that you mention the Views thing. That feels like Imagecache in Drupal 6 :D
I will open a new issue with the "Views thing" for Drupal 8 of it's not in core yet.
Comment #72
David_Rothstein CreditAttribution: David_Rothstein commentedSeems like we should move this back to "needs review" while we figure all that out.
Interesting point - thanks!
Comment #73
yannickooCreated a followup issue with the patch #1875002: Allow other modules to alter field formatter settings in views fields.
Comment #74
klonos...friendly bump ;)
Comment #75
Dave ReidSo I would definitely but shutting down the field_formatter_settings module if this were accepted into core. I would have to wait until the next release of core in order to update the .info file of the module to specify that a version less than Drupal 7.2? is necessary to use the module and would error if you try to use the module with a higher version of core.
As for the hooks, I don't see any reason why having both the core and the contrib module invoking the same hooks.
For hook_field_formatter_settings_form_alter(), having it invoked twice means that the same form elements get written in. I haven't seen a use case where these form elements don't have hard-coded key values, so they would just get overridden on the second invocation.
For hook_field_formatter_settings_summary_alter(), having it invoked twice would mean that the summary possibly includes duplicated information. But it doesn't affect functionality and will not cause any bugs. This would be a good key to the user that they need to disable the module.
Comment #76
andrewmacpherson CreditAttribution: andrewmacpherson commentedAs well as the two hooks, the D7 contrib API module provides a helper function to get the formatter settings:
field_formatter_settings_get_instance_display_settings()
Some of the field_formatter_settings modules ditched their own helper functions in favour of this.
We'd need to include this in D7 core along with the hooks, otherwise modules will have to go back to implementing their own helper function again.
It's not an issue for D8, because entity_get_render_display() is available.
Comment #77
webchickSince Drupal.org D7 upgrade needs this in order to optimally solve #1628044: Implement magic for the table of attachments on issues, which is a critical task, moving this to a task rather than a feature.
However, since just adding a dependency on http://drupal.org/project/field_formatter_settings in the meantime doesn't seem the end of the world, keeping it as just a "normal" task, rather than a major/critical one. If that's incorrect, feel free to bump accordingly.
Comment #78
dwwCorrect, it'd make life better if we had this, but we don't technically need it. Therefore, not major/critical.
Thanks,
-Derek
Comment #79
andrewmacpherson CreditAttribution: andrewmacpherson commentedAlthough this got committed way, no Change Notice was written. I discussed this with Swentel in Dublin at Drupal Dev Days, and I'll write the change notice with examples.
Comment #80
andrewmacpherson CreditAttribution: andrewmacpherson commentedComment #81
andrewmacpherson CreditAttribution: andrewmacpherson commentedHere is a first draft of a D8 change notice for the two new hooks. Changing version to D8for now (can change back to D7 and reassign to Dave Reid once the change notice is agreed.)
----------
As of Drupal 8.x, two new alter hooks are available in Field API:
hook_field_formatter_settings_summary_alter()
hook_field_formatter_settings_form_alter()
The new hooks allow developers to alter the field formatter settings form in Field UI. A typical use-case is to extend a field formatter with a new setting, which can be used later when rendering the field, e.g. in hook_preprocess_field().
These hooks were previously provided in Drupal 7.x by the Field Formatter Settings API module in Contrib.
Usage
In this example, we will add a checkbox setting to a field formatter, and retrieve the setting in hook_preprocess_field().
Drupal 7
(requires Field Formatter Settings API module)
Drupal 8
Notes
$summary
variable was passed intohook_field_formatter_settings_summary_alter()
as a string. It was typically necessary to add a <br> to keep the summary display tidy.In Drupal 8.x, the
$summary
variable is an array passed by reference, so there is no need to add the <br> manually.hook_field_formatter_settings_form_alter()
has changed in Drupal 8.xIn Drupal 8.x, the field formatter settings can be retrived using entity_get_render_display() and EntityDisplayBase::getComponent().
Comment #82
swentel CreditAttribution: swentel commented@andrewmacpherson Can you create the change notice, looks good to me, then we can close this one as I'm pretty sure it's better to just keep the formatter settings module in contrib for D7.
Comment #83
Summit CreditAttribution: Summit commentedHi,
For drupal 7, in which module this https://drupal.org/node/945524#comment-7587155 will be integrated please?
Coming from: https://drupal.org/project/linked_field this hooks are necessary, right?
Thanks for your reply in advance! Greetings, Martijn
Comment #84
andypost#81 has draft that should be posted
Comment #85
jthorson CreditAttribution: jthorson commented#83 Summit: For Drupal 7, use the Field Formatter Settings module.
Comment #86
jthorson CreditAttribution: jthorson commentedd.o tag monster.
Comment #87
swentel CreditAttribution: swentel commentedChange notifications are major
Comment #88
andrewmacpherson CreditAttribution: andrewmacpherson commentedI added the change record from #81, with a minor change in the first sentence to make it clear the hooks are in Field UI API (rather than Field API).
https://drupal.org/node/2130757
Closing issue, per #82
Comment #89
andrewmacpherson CreditAttribution: andrewmacpherson commentedComment #90
klonosBTW, we should stop hijacking tags and add a "Needs change notification" status.
Comment #91
andrewmacpherson CreditAttribution: andrewmacpherson commented@klonos - that is a wonderful idea! "Needs change notification" has become a part of our mainstream workflow.
The best place to suggest this would be in the drupal.org infrastructure issuequeue.
Vvv
Comment #91
andrewmacpherson CreditAttribution: andrewmacpherson commented@klonos - that is a wonderful idea! "Needs change notification" has become a part of our mainstream workflow.
The best place to suggest this would be in the drupal.org infrastructure issuequeue.
Vvv
Comment #92
andrewmacpherson CreditAttribution: andrewmacpherson commented@klonos - that is a wonderful idea! "Needs change notification" has become a part of our mainstream workflow.
The best place to suggest this would be in the drupal.org infrastructure issuequeue.
Vvv
Comment #92
andrewmacpherson CreditAttribution: andrewmacpherson commented@klonos - that is a wonderful idea! "Needs change notification" has become a part of our mainstream workflow.
The best place to suggest this would be in the drupal.org infrastructure issuequeue.
Vvv
Comment #93
andrewmacpherson CreditAttribution: andrewmacpherson commented@klonos - that is a wonderful idea! "Needs change notification" has become a part of our mainstream workflow.
The best place to suggest this would be in the drupal.org infrastructure issue queue.
Comment #94
andrewmacpherson CreditAttribution: andrewmacpherson commentedOkay that was weird. Sorry about duplicate comments. Timeouts on a flaky connection ;-(
Comment #95
klonos@andrewmacpherson: #2133257: Add a "Needs change notification" status and stop hijacking the issue tags system. ;)
Comment #96
jibranFixing title and tags.
Comment #97
andrewmacpherson CreditAttribution: andrewmacpherson as a volunteer and at Annertech commented