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.
When a field is set to "hidden' in "Manage form displays", the EntityFormDisplay CMI record stores no entry for the field in it 'content' property.
So $form_display->getComponent($field_name) returns NULL,
and $form_display->getRenderer($field_name) just returns NULL, not a HiddenWidget plugin object.
This is perfectly fine by me, and is in line with what happens with 'hidden' fields on the display side.
Why don't we stay that way and remove the "hidden" widget then ?
The change notice at https://drupal.org/node/1968224 can then be updated to just say "fields can be set to hidden in forms".
Comment | File | Size | Author |
---|---|---|---|
#32 | field_remove_hidden_widget-2060705-32.patch | 4.74 KB | amateescu |
#30 | field_remove_hidden_widget-2060705-30.patch | 4.82 KB | swentel |
#30 | interdiff.txt | 609 bytes | swentel |
#28 | field_remove_hidden_widget-2060705-28.patch | 4.82 KB | swentel |
#27 | field_remove_hidden_widget-2060705-27.patch | 4.85 KB | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedbetter title
Comment #2
amateescu CreditAttribution: amateescu commentedThis is a bug, it did return the hidden widget plugin at some point in either the EntityFormDisplay patch or the form modes one, but it's definitely the form modes patch that lost it..
IMO, because we would have to hardcode the 'hidden' option in several places. One random example that comes to mind would be something like Rules, which could display a list of widget plugins for a condition that checks how a field is displayed in an entity form.
Comment #3
jpstrikesback CreditAttribution: jpstrikesback commentedMmm, I hate to chime in without understanding CMI but before this ship sails, perhaps it is because getDefaultValueWidget is still trying to find $this->instance['widget']?
#2028759: Clean up instance['widget'] in Field UI
Comment #4
amateescu CreditAttribution: amateescu commentedThat's being fixed in another issue (#2056405: Let field types provide their support for "default values" and associated input UI), and this discussion is not at all about CMI..
Comment #5
yched CreditAttribution: yched commentedTBH, I think I prefer the special casing of 'hidden'. We can't have a "hidden formatter" for perf reasons, and the break of consistency between EntityDisplays on form and EntityDisplays on display is a source of troubles and makes maintaining the system harder IMO.
Comment #6
yched CreditAttribution: yched commentedHmm, it's not being fixed in #2056405: Let field types provide their support for "default values" and associated input UI, I'm on that issue right now :-)
Comment #7
amateescu CreditAttribution: amateescu commentedI was talking about this part:
$this->instance['widget']
that's being fixed there. At least I saw it was removed in a patch from several days ago, didn't check since then.I think the performance issues can be overcome, probably by instantiating the hidden formatter only once and keeping it "cached" in EntityDisplay. But I dunno, as you prefer..
Comment #8
jpstrikesback CreditAttribution: jpstrikesback commented@amateescu, indeed, I wrote without grokking...
Comment #9
yched CreditAttribution: yched commented"instantiating the hidden formatter only once and keeping it "cached" in EntityDisplay"
The issue would be then calling the view() method on a class that does nothing for N hidden fields in M entity teasers...
Comment #10
amateescu CreditAttribution: amateescu commentedRight, no more objections from me then.
Comment #11
yched CreditAttribution: yched commentedre: $this->instance['widget']
Yeah, sorry, there was a series of crossposts. So, yes, this is fixed in #2056405: Let field types provide their support for "default values" and associated input UI, but isn't related to "hidden widget plugin not being called in regular entity forms".
Comment #12
amateescu CreditAttribution: amateescu commentedYep, it's not related, I was just replying to #3.
Comment #13
yched CreditAttribution: yched commentedTemptative patch.
Comment #14
yched CreditAttribution: yched commentedBump + reroll.
Comment #15
swentel CreditAttribution: swentel commentedMakes sense indeed. Tested it manually by adding a default value to a textfield, moving to hidden on form display and then creating content, it still works.
One thing I've wondering though: in #2014821: Introduce form modes UI and their configuration entity we removed following line:
But we didn't add a replacement for it. Not sure whether we need to bring that back, but probably not in this issue.
Comment #16
yched CreditAttribution: yched commented@swentel: yes, I was also wondering where that warning went. I do think we would need it back, but that sounds like a separate followup for #2014821: Introduce form modes UI and their configuration entity
Comment #18
swentel CreditAttribution: swentel commentedtagging
Comment #19
Hydra CreditAttribution: Hydra commentedOkay, lets start with a reroll. But locally it looked like it's not working. Also some more tests are failing, confirming my manual tests. This may needs some more work.
Comment #20
Hydra CreditAttribution: Hydra commentedUpdated patch
Comment #21
yched CreditAttribution: yched commentedMerged on my local sandbox, fixes a couple merge issues.
Comment #22
swentel CreditAttribution: swentel commentedCool!
Comment #23
swentel CreditAttribution: swentel commentedWe can obsolete (or maybe delete?) https://drupal.org/node/1968224 and add it to the change notice for form modes.
Comment #24
yched CreditAttribution: yched commentedMaybe we should just transform it to "fields can be hidden in entity forms" ?
Comment #25
swentel CreditAttribution: swentel commentedSounds good to me.
Comment #26
alexpottPatch no longer applies.
Comment #27
yched CreditAttribution: yched commentedReroll
Comment #28
swentel CreditAttribution: swentel commentedRerolled
Comment #30
swentel CreditAttribution: swentel commentedduh
Comment #31
alexpottPatch no longer applies.
Comment #32
amateescu CreditAttribution: amateescu commentedSad reroll :/
Comment #33
catchiirc we added this before form display modes got in. Fine with removing it if it's not used - nothing stops it being added back if there's a need for it. Committed/pushed to 8.x, thanks!
Comment #34
jibranI think we should create/update change notice.
Comment #35
amateescu CreditAttribution: amateescu commentedI think we just need to delete this one: https://drupal.org/node/1968224
Comment #36
anavarre#35 Looks like the way to go.
Comment #37
catchUpdated [#1968224].
Comment #38.0
(not verified) CreditAttribution: commentedchange notice will need update