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".

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

yched’s picture

Title: The 'hidden' widget is actually never used - remove it ? » The 'hidden widget' plugin is actually never used - remove it ?

better title

amateescu’s picture

$form_display->getRenderer($field_name) just returns NULL

This 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..

Why don't we stay that way and remove the "hidden" widget then ?

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.

jpstrikesback’s picture

Mmm, 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

amateescu’s picture

That'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..

yched’s picture

TBH, 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.

yched’s picture

Hmm, 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 :-)

amateescu’s picture

I 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..

jpstrikesback’s picture

@amateescu, indeed, I wrote without grokking...

yched’s picture

"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...

amateescu’s picture

Right, no more objections from me then.

yched’s picture

re: $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".

amateescu’s picture

Yep, it's not related, I was just replying to #3.

yched’s picture

Status: Active » Needs review
FileSize
4.79 KB

Temptative patch.

yched’s picture

Title: The 'hidden widget' plugin is actually never used - remove it ? » Remove the 'hidden widget' plugin, it is actually never used
FileSize
4.79 KB

Bump + reroll.

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Makes 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:

if ($this->instance['required'] && empty($this->instance['default_value']) && empty($this->instance['default_value_function']) && $this->instance['widget']['type'] == 'field_hidden') {
-      drupal_set_message(t('Field %label is required and uses the "hidden" widget. You might want to configure a default value.', array('%label' => $this->instance['label'])), 'warning');
-    }

But we didn't add a replacement for it. Not sure whether we need to bring that back, but probably not in this issue.

yched’s picture

@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

Status: Reviewed & tested by the community » Needs work

The last submitted patch, field_remove_hidden_widget-2060705-14.patch, failed testing.

swentel’s picture

Issue tags: +Field API

tagging

Hydra’s picture

Status: Needs work » Needs review
FileSize
5.16 KB

Okay, 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.

Hydra’s picture

Updated patch

yched’s picture

Merged on my local sandbox, fixes a couple merge issues.

swentel’s picture

Status: Needs review » Reviewed & tested by the community

Cool!

swentel’s picture

We can obsolete (or maybe delete?) https://drupal.org/node/1968224 and add it to the change notice for form modes.

yched’s picture

Maybe we should just transform it to "fields can be hidden in entity forms" ?

swentel’s picture

Sounds good to me.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

yched’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
FileSize
4.85 KB

Reroll

swentel’s picture

Status: Reviewed & tested by the community » Needs work

The last submitted patch, field_remove_hidden_widget-2060705-28.patch, failed testing.

swentel’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
609 bytes
4.82 KB

duh

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs reroll

Patch no longer applies.

amateescu’s picture

Status: Needs work » Reviewed & tested by the community
Issue tags: -Needs reroll
FileSize
4.74 KB

Sad reroll :/

catch’s picture

Status: Reviewed & tested by the community » Fixed

iirc 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!

jibran’s picture

Title: Remove the 'hidden widget' plugin, it is actually never used » Change notice: Remove the 'hidden widget' plugin, it is actually never used
Priority: Normal » Major
Status: Fixed » Active
Issue tags: +Needs change record

I think we should create/update change notice.

amateescu’s picture

I think we just need to delete this one: https://drupal.org/node/1968224

anavarre’s picture

#35 Looks like the way to go.

catch’s picture

Title: Change notice: Remove the 'hidden widget' plugin, it is actually never used » Remove the 'hidden widget' plugin, it is actually never used
Status: Active » Fixed
Issue tags: -Needs change record

Updated [#1968224].

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

Anonymous’s picture

Issue summary: View changes

change notice will need update