Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Form][2.7] Allow choices with duplicated content#16582

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Closed
ewgRa wants to merge1 commit intosymfony:2.7fromewgRa:issue-15606-2.7
Closed

[Form][2.7] Allow choices with duplicated content#16582

ewgRa wants to merge1 commit intosymfony:2.7fromewgRa:issue-15606-2.7

Conversation

@ewgRa
Copy link
Contributor

QA
Bug fix?yes
New feature?no
BC breaks?no, but not sure
Deprecations?no
Tests pass?yes
Fixed tickets#15606
LicenseMIT
Doc PR

@ewgRaewgRa changed the titleAllow choices with duplicated content[Form][2.7] Allow choices with duplicated contentNov 18, 2015
@ewgRa
Copy link
ContributorAuthor

Test fails seems not related.

ping@webmozart

@symfony/deciders is it possible to have your point of view on this ?

Choose a reason for hiding this comment

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

adding a method to an interface is a BC break

Choose a reason for hiding this comment

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

maybe you could add a new interface instead, with only these methods

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Ok, I will check.

@stof
Copy link
Member

What is the use case for having duplicate choices ?
For duplicate labels, this can be resolved with the existing system by usingchoice_label to build labels for each choice instead of relying on the default behavior

@ewgRa
Copy link
ContributorAuthor

@stof you can check#15606 for cases, there is some. Translations cases, grouped choices with same options in groups.

What is a choice_label is? Can you give a link?

Also it is a bug. To say people than it is "can be resolved by" - it must be documented as feature then. No?

@Tobion
Copy link
Contributor

What@stof means and what I agree with is that duplicate choices (what you would get as selected value) makes no sense as you would not be able to distinguish each one anyway. Having the same label can happen and this is possible with thechoice_label option.
The problem in#15606 is that the BC layer for the old choices structure (whenchoices_as_values: false) flips the choices which of course removes all choices which had the same label. So this BC layer must be fixed. So the current solution seems wrong.

@ewgRa
Copy link
ContributorAuthor

@Tobion first of all thanks for review

makes no sense as you would not be able to distinguish each one anyway

Sometimes it is not needed, sometimes business want to show same thing twice, just to increase chance that user select it, or do not miss.

Also as check my case:#15606 (comment).

About choice_label, if you mean something like this:

    public function testCreateViewDuplicateArrayKeyChoiceListValuesWithChoiceLabel()    {        $list = new ArrayKeyChoiceList(            array(                'A' => 'a',                'AA' => 'a'            )        );        $view = $this->factory->createView(            $list,            null,            function ($allChoices, $currentChoiceKey) {                return 'a';            }        );        $this->assertEquals(            new ChoiceListView(                array(                    0 => new ChoiceView('A', 'A', 'a'),                    1 => new ChoiceView('AA', 'AA', 'a')                ),                array()            ),            $view        );    }

It is not working.

The problem in#15606 is that the BC layer for the old choices structure (when choices_as_values: false)
So this BC layer must be fixed

Can you check tests from this PR? For example there is a test that create ArrayKeyChoiceList without second argument, and in this case useChoicesAsValues will be true. It is not only about BC layer as I understand.

Would be nice to hear@webmozart, as contributor of this code.

@webmozart
Copy link
Contributor

Hi :) Thanks for working on this! I think this should be fixed in a less intrusive way. See#16685.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ewgRa@stof@Tobion@webmozart@nicolas-grekas@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp