Problem/Motivation

When using field formatters in slots, the states API is not working correctly.

I noticed FieldFormatterSource::generateFieldFormatterForm mentions

// Should we use FormHelper::rewriteStatesSelector() ?
// like in FieldBlock::formatterSettingsProcessCallback.

I think it should!

We can find other formatters using states by searching for [name="fields[ and [name='fields[

Steps to reproduce

- Install demo_umami, enable ui_patterns
- Go to /admin/structure/types/manage/article/display/full/layout
- Add a ui_patterns block, like Title, use a timestamp field formatter inside a slot
-- Source: [Entity] ➜ [Field]
-- Field: Changed
-- Source: [Field] Formatter
-- Formatter: Default
- Notice states is not working for "Display as a time difference"

Proposed resolution

Rewrite states, as suggested in code comments.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

herved created an issue. See original summary.

herved’s picture

Issue summary: View changes

herved’s picture

Issue summary: View changes

Created MR, this solves states not working, but the "enabled" key is not saved when using the TimestampFormatter, causing PHP notices.

Warning: Undefined array key "enabled" in Drupal\Core\Field\Plugin\Field\FieldFormatter\TimestampFormatter->viewElements() (line 319 of core/lib/Drupal/Core/Field/Plugin/Field/FieldFormatter/TimestampFormatter.php).

It looks like there is something not quite right with how FieldFormatterSource builds the form/subform.
Maybe we need to compare and check exactly how \Drupal\layout_builder\Plugin\Block\FieldBlock does it.

Edit: Ooof, not an easy debug, \Drupal\ui_patterns\Element\ComponentForm::elementValidate is very suspicious and seems to cause the loss of that checkbox value.
Before drupal hits this method, the checkbox value is in form state, but then that method overrides the values in the form state via $form_state->setValueForElement(). That code is very odd and likely exists to work around another underlying bug.

herved’s picture

Unfortunately there is no test coverage for this ComponentForm::elementValidate
So I'm not sure what to make of it...
Looking at the gitlog, it originates from:
- https://git.drupalcode.org/project/ui_patterns/-/commit/509f99793542456e...
- https://git.drupalcode.org/project/ui_patterns/-/commit/b2216dcb5daa8653...

Moving to review to get some feedback.

herved’s picture

Status: Active » Needs review
herved’s picture

Status: Needs review » Needs work
herved’s picture

Status: Needs work » Needs review

Could you please review this? It seems there are 2 issues:
1. states api not working
2. and I made an attempt for the ComponentForm::elementValidate and related code

herved’s picture

Status: Needs review » Needs work

Moving back to needs work, the LB configure form closes unexpectedly with this when adding something in a slot.
I have to dig further, possibly split off the 2nd issue.

herved’s picture

Status: Needs work » Needs review

It seems the main goal of ComponentSlotForm::elementValidate is to remove some values (add_more_button, _remove) from the final submitted form values which ultimately ends up in config.

I believe form elements ['#value'] should not be changed like this.
Nor should $form_state->setValueForElement() be used on the whole component_form element in ComponentForm::elementValidate otherwise we loose such empty checkbox values which are present in form_state.
But instead use $form_state->unsetValue() in ComponentSlotForm::elementValidate.
There are some examples in core where this pattern is used to cleanup unwanted values in form_state.
So I updated accordingly.

I'm not 100% sure on this or that it won't have side effects in ui_patterns so a thorough testing is needed.
I do see some calls to $form_state->setValueForElement() in ui_patterns_ui.

herved’s picture

phpmd job seems rogue

just_like_good_vibes’s picture

Assigned: Unassigned » just_like_good_vibes

hello,
thank you for reporting that error and for contributing.
we will check this soon.