Problem/Motivation

This can be a method on FormState, preventing code from having to call out to the form builder.

Proposed resolution

Remove the method from FormBuilder
Leave the deprecated form_set_value() function in place, it will be removed by an issue in #2205673: [META] Remove all @deprecated functions marked "remove before 8.0"

Remaining tasks

User interface changes

API changes

CommentFileSizeAuthor
#6 form_state-2313397-6.patch9.76 KBtim.plunkett
#3 interdiff.txt960 bytestim.plunkett
#3 form_state-2313397-3.patch9.92 KBtim.plunkett
#1 form_state-2313397-1.patch8.78 KBtim.plunkett

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Active » Needs review
StatusFileSize
new form_state-2313397-1.patch8.78 KB
jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented

I haven't seen any test code for this method. I think we should add some quick unit test for it.

+++ b/core/lib/Drupal/Core/Form/FormState.php
@@ -7,6 +7,7 @@
+use Drupal\Component\Utility\NestedArray;

Are we still using it in FormBuilde? Perhaps we can remove it.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
StatusFileSize
new form_state-2313397-3.patch9.92 KB
new interdiff.txt960 bytes

We are still using NestedArray in FormBuilder for other things.
Here's a test.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Thank you RTBC.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs change record updates, +Needs reroll

Needs a reroll plus can someone work out which CRs need updating.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs work » Needs review
Issue tags: -Needs reroll
StatusFileSize
new form_state-2313397-6.patch9.76 KB

Here's the reroll, will find the CR next.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Title: Replace FormBuilderInterface::setVaue() with FormStateInterface::setValueForElement() » Replace FormBuilderInterface::setValue() with FormStateInterface::setValueForElement()
Issue tags: -Needs change record updates

Hmm, there were no CRs pointing to FormBuilderInterface::setValue(), and none about form_set_value(), which still exists.
So this either needs one now, or we write one when we kill the actual functions.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Toronto, Canada
commented
Status: Needs review » Reviewed & tested by the community

Thanks for the reroll. Back to RTBC.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented

Posted https://www.drupal.org/node/2315243 now instead of later.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

I deleted https://www.drupal.org/node/2315243 and added this to https://www.drupal.org/node/2121003

webchick’s picture

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

Well that's definitely much nicer.

Committed and pushed to 8.x. Thanks!

  • webchick committed 73a65a3 on 8.0.x 
    Issue #2313397 by tim.plunkett: Replace FormBuilderInterface::setValue...
tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Assigned: tim.plunkett » Unassigned

Thanks!

Status: Fixed » Closed (fixed)

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