[Form] added "choice_label_attr" option and deprecated "choice_attr" as multi-arrays or property path #16834

HeahDude · 2015-12-04T12:58:13Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	yes
Tests pass?	no
Fixed tickets	#14404, #18318
License	MIT
Doc PR	todo

68c1557 deprecates choice_attr as nested arrays in favor of a global array for all choices or a callable,
f012ebe deprecates choice_attr as string, property path,
f7649d1 adds choice_label_attr option for radio buttons and
checkboxes which can be a global array for all choices or a callable.
Address a doc PR
Update CHANGELOG

HeahDude · 2016-02-22T15:01:55Z

This one has been approved a long time ago by @webmozart in #14413 (comment).

My original use case was a complex form with many nested ChoiceType expanded and the need to add custom classes to option labels. It actually requires complex loops in twig and maybe override form themes.

But it can be very helpful for EntityType which inherits this option and add some consistency with choice_value, choice_name, choice_label and choice_attr introduced in #14050.

Here's a basic usage :

use AppBundle\Entity\Task;
use Symfony\Bridge\Doctrine\Form\Type\EntityType;

$form = $this->createFormBuilder()
    // ...
    ->add('tasks', EntityType::class, array(
        'class' => Task::class,
        'expanded' => true,
        'choice_label' => 'todo',
        'choice_label_attr' => function($task) {
            if (null === $task || '' === $task) {
                return;
            }

            $class = 'label-task';

            if ($task->needsReview) {
                $class .= ' badged-label badge-review';
            } elseif ($task->isImportant || $task->dueDate() < new \DateTime('+ 2 days')) {
                $class .= ' font-red badged-label badge-alert';
            }

            return array('class' => $class);
        },
        // ...
    );

Is there any hope to get this one merged in 3.1 ?

ping @symfony/deciders

fabpot · 2016-03-04T09:35:48Z

@webmozart Could you validate this new feature? It looks like quite straightforward but having your approval would probably help. Thanks.

HeahDude · 2016-03-06T12:31:27Z

Saw that question on stack today : http://stackoverflow.com/questions/35769809/how-to-style-choice-field-labels-in-symfony/35826837#35826837

xabbuh · 2016-03-07T12:12:09Z

src/Symfony/Component/Form/ChoiceList/Factory/ChoiceListFactoryInterface.php

     * @return ChoiceListView The choice list view
     */
-    public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null);
+    public function createView(ChoiceListInterface $list, $preferredChoices = null, $label = null, $index = null, $groupBy = null, $attr = null, $labelAttr = null);


This is a BC break.

webmozart · 2016-03-07T14:35:15Z

Technically, this feature is fine, but after the changes to our BC policy in symfony/symfony-docs#5735 I don't see how this can be added without breaking BC.

HeahDude · 2016-03-07T14:38:00Z

It's the same BC break as introduced in 2.7. I was too late at this time :(

HeahDude · 2016-03-07T15:53:19Z

If I understand this right, it is a matter of choice between two edge cases: either the one extending choice factories or implementing the factory interface, or the one needing this feature.

Actually is a dev able to implement this feature on its own without messing with the core interface ?

HeahDude · 2016-03-26T06:09:15Z

Complete refactoring, see #18318. Updated description.

HeahDude · 2016-03-26T06:09:51Z

Failures are unrelated.

HeahDude · 2016-11-10T00:00:14Z

Rebased and green :) Would you merge this one?

fabpot · 2016-11-10T00:16:58Z

That would be for 3.3 :)

Sander-Toonen · 2017-03-21T20:32:34Z

Is this still planned for 3.3? Really looking forward to see this merged.

fabpot · 2017-03-22T16:55:17Z

@HeahDude What's the status of this PR? Last call for 3.3 :)

HeahDude · 2017-03-26T18:40:13Z

@fabpot I've updated the changelogs, upgrade file and deprecations notices to target 3.3, and also pushed a fourth commit 0fe7097, to remove the usage of an extra interface in favor of a proper deprecation, if you agree with this change I'll add some tests and the corresponding changelog entry.

HeahDude · 2017-03-26T18:55:34Z

src/Symfony/Component/Form/Extension/Core/Type/ChoiceType.php

+        // BC
+        $refMethod = new \ReflectionMethod($this->choiceListFactory, 'createView');
+        if (6 > $refMethod->getNumberOfParameters()) {
+            @trigger_error(sprintf('Not passing a "$labelAttr" as sixth argument of "%s" is deprecated since version 3.3 and will trigger an error in 4.0.', $refMethod->getNamespaceName()));


In fact the message should say "Not declaring the seventh argument ..."

@symfony/deciders Should I go with this bc layer to update the interface in 4.0 rather than adding the extra interface (cf last commit)?

stof · 2017-03-31T11:12:16Z

src/Symfony/Component/Form/ChoiceList/Factory/PropertyAccessDecorator.php

            @trigger_error('Passing callable strings is deprecated since version 3.1 and PropertyAccessDecorator will treat them as property paths in 4.0. You should use a "\Closure" instead.', E_USER_DEPRECATED);
        }

+        // Deprecated since 3.3 and to be removed in 4.0 with the condition above


if it is deprecated, it must trigger a deprecation notice.

And why deprecating this feature ? It is not consistent with other properties.

Because of what =I explained in the related issue, it's more consistent IMO to deprecate this behavior rather than supporting it for choice_label_attr too, this is unneeded overhead, the callable or unique array should be the way to go for these.

lrlopez · 2017-04-09T23:15:55Z

UPGRADE-3.3.md

+   },
+   ```
+
+ * Using `choice_attr` option as a string or a `ProprertyPath` instance has been


Nice catch, thanks!

lrlopez · 2017-04-09T23:16:47Z

UPGRADE-4.0.md

+   },
+   ```
+
+ * Using `choice_attr` option as a string or a `ProprertyPath` instance will


nicolas-grekas · 2017-09-26T11:59:46Z

@HeahDude what do you want to do with this PR? Last call for deprecations in 3.4 :)

fabpot · 2018-01-24T13:28:42Z

@HeahDude Do you think you can work on finishing this PR or shall we close it?

HeahDude · 2018-01-25T00:38:25Z

Don't worry I'll finish all my opened PRs, they're just not the priority for now. I've made a clean up by closing some for the last feature freeze, not having time to finish anything.
2.7 is still my first target with missing fixes and tests, continuing #21877 and closing old issues, another bunch before I finish here.
Happy rebasing and conflict merging stuff are ahead this year <3

fabpot · 2018-03-31T05:59:01Z

Closing this PR as there is no more activity. @HeahDude Feel free to reopen whenever you have time to finish it. Thanks.

FloatingMaster · 2021-09-02T14:03:41Z

This feature would be quite useful with ChoiceType multiple=true and expanded=true. Sad it wasn't merged.

carsonbot added the Status: Needs Review label Dec 4, 2015

HeahDude mentioned this pull request Dec 4, 2015

[2.8] [Form] Feature choice label attr #15887

Closed

carsonbot added Status: Needs Work Status: Needs Review and removed Status: Needs Review Status: Needs Work labels Dec 4, 2015

HeahDude mentioned this pull request Jan 20, 2016

[Form] Add "choice_labels", "choice_values", "choice_attr" and "group_by" options to ChoiceType #4067

Closed

HeahDude mentioned this pull request Feb 4, 2016

[Form] Add option "widget" to ChoiceType #6602

Closed

HeahDude force-pushed the feature-add_choice_label_attr branch from 7a990a8 to 8fc661a Compare February 20, 2016 05:01

javiereguiluz added Form Feature labels Mar 3, 2016

xabbuh reviewed Mar 7, 2016
View reviewed changes

HeahDude changed the title ~~[3.1][Form] Feature : add choice_label_attr option to ChoiceType~~ [Form] add choice_label_attr option to ChoiceType Mar 7, 2016

webmozart mentioned this pull request Mar 7, 2016

[Contributing][Code] do not distinguish regular classes and API classes symfony/symfony-docs#5735

Merged

javiereguiluz added the BC Break label Mar 7, 2016

HeahDude force-pushed the feature-add_choice_label_attr branch from 8fc661a to 104f542 Compare March 8, 2016 05:50

HeahDude force-pushed the feature-add_choice_label_attr branch from 104f542 to 6f71cdc Compare March 26, 2016 05:57

HeahDude mentioned this pull request Mar 26, 2016

[Form] [DX] choice_attr and choice_label_attr options in ChoiceType #18318

Open

HeahDude force-pushed the feature-add_choice_label_attr branch 2 times, most recently from 0d69e63 to 7bd6d19 Compare March 26, 2016 18:08

HeahDude force-pushed the feature-add_choice_label_attr branch from c18aaf8 to aa9c3cc Compare November 9, 2016 23:48

HeahDude force-pushed the feature-add_choice_label_attr branch 2 times, most recently from 648c8a7 to aa4d19a Compare November 10, 2016 00:14

nicolas-grekas added this to the 3.x milestone Dec 6, 2016

HeahDude added 3 commits March 26, 2017 20:30

[Form] deprecated choice_attr as nested arrays mapped by indexes

e5cc7cd

[Form] deprecated "choice_attr" option as PropertyPath

eb658f9

[Form] add choice_label_attr option to ChoiceType

23df93d

HeahDude force-pushed the feature-add_choice_label_attr branch from aa4d19a to 9d9301e Compare March 26, 2017 18:35

Fixed choice_label_attr option BC layer

0fe7097

HeahDude force-pushed the feature-add_choice_label_attr branch from 9d9301e to 0fe7097 Compare March 26, 2017 18:46

HeahDude commented Mar 26, 2017

View reviewed changes

stof reviewed Mar 31, 2017

View reviewed changes

lrlopez reviewed Apr 9, 2017

View reviewed changes

UPGRADE-4.0.md

},

```

* Using `choice_attr` option as a string or a `ProprertyPath` instance will

Copy link

Contributor

lrlopez Apr 9, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

HeahDude mentioned this pull request Sep 4, 2017

choice_attr doesn't work for ChoiceType expanded multiple #24088

Closed

nicolas-grekas modified the milestones: 3.4, 4.1 Oct 8, 2017

fabpot closed this Mar 31, 2018

HeahDude mentioned this pull request Mar 30, 2020

[Form] Deprecated choice_attr as nested arrays mapped by indexes #36272

Closed

xabbuh mentioned this pull request Dec 1, 2021

[TwigBridge] label_attr not work #44332

Closed

Uh oh!

[Form] added "choice_label_attr" option and deprecated "choice_attr" as multi-arrays or property path #16834

[Form] added "choice_label_attr" option and deprecated "choice_attr" as multi-arrays or property path #16834

Uh oh!

Conversation

HeahDude commented Dec 4, 2015 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HeahDude commented Feb 22, 2016

Uh oh!

fabpot commented Mar 4, 2016

Uh oh!

HeahDude commented Mar 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

webmozart commented Mar 7, 2016

Uh oh!

HeahDude commented Mar 7, 2016

Uh oh!

HeahDude commented Mar 7, 2016

Uh oh!

HeahDude commented Mar 26, 2016

Uh oh!

HeahDude commented Mar 26, 2016

Uh oh!

HeahDude commented Nov 10, 2016

Uh oh!

fabpot commented Nov 10, 2016

Uh oh!

Sander-Toonen commented Mar 21, 2017

Uh oh!

fabpot commented Mar 22, 2017

Uh oh!

HeahDude commented Mar 26, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolas-grekas commented Sep 26, 2017

Uh oh!

fabpot commented Jan 24, 2018

Uh oh!

HeahDude commented Jan 25, 2018

Uh oh!

fabpot commented Mar 31, 2018

Uh oh!

FloatingMaster commented Sep 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

11 participants

HeahDude commented Dec 4, 2015 •

edited

Loading

HeahDude commented Mar 26, 2017 •

edited

Loading