8000 Update ChoiceFormField.php by fabpot · Pull Request #15952 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

Update ChoiceFormField.php #15952

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.

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Update ChoiceFormField.php #15952

wants to merge 1 commit into from

Conversation

fabpot
Copy link
Member
@fabpot fabpot commented Sep 28, 2015
Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

This PR was submitted on the symfony/dom-crawler read-only repository by @bouland and moved automatically to the main Symfony repository (closes symfony/dom-crawler#20).

Hi,

A select could be "globaly" disabled not only one of its options.
http://www.w3schools.com/tags/att_select_disabled.asp

@bouland @fabpot
Update ChoiceFormField.php
4692d46 
Hi,

A select could be "globaly" disabled not only one of its options.
http://www.w3schools.com/tags/att_select_disabled.asp
@xabbuh
Copy link
Member
xabbuh commented Sep 28, 2015

Imo this deserves a test to avoid regressions.

Status: Needs work

@stof
Copy link
Member
stof commented Sep 28, 2015

I agree with @xabbuh

@fabpot
Copy link
Member Author
fabpot commented Oct 6, 2015

@bouland Can you work on adding some unit tests?

@bouland
Copy link
Contributor
bouland commented Oct 7, 2015

I was a little lost, I didn't understand what was expected from me :
"Some changes should be done to comply with our standards."
I will add tests soon.

@xabbuh
Copy link
Member
xabbuh commented Oct 7, 2015

@bouland The pull request header (the table in the initial description was missing). I added it as you don't have the necessary permission to edit @fabpot's post.

@fabpot
Copy link
Member Author
fabpot commented Jan 25, 2016

@bouland Any news on this one?

@bouland
Copy link
Contributor
bouland commented Jan 25, 2016

Well i have not taken time to write the test.
Give me a week more.

@bouland
Copy link
Contributor
bouland commented Jan 25, 2016

Could you give me push access to the PR branch
fabpot:moved_domcrawler_20
thx

@fabpot
Copy link
Member Author
fabpot commented Jan 25, 2016

I can't. The best would be to close this PR and open a new one.

@fabpot fabpot closed this Jan 25, 2016
@xabbuh
Copy link
Member
xabbuh commented Jan 25, 2016

@bouland By the way, how did you end up with having a ChoiceFormField object pointing to an option element?

@bouland bouland mentioned this pull request Jan 26, 2016
Closed
@bouland
Copy link
Contributor
bouland commented Jan 26, 2016

When i want to write a WebTestCase which submit a Form with a select with attribute disabled.

it calls $form->getPhpValues()

public function Client::submit(Form $form, array $values = array())
{
     $form->setValues($values);

     return $this->request($form->getMethod(), $form->getUri(), $form->getPhpValues(), $form->getPhpFiles());
}

then the test $field->isDisabled() didn't work with ChoiceFormField of type "select"

public function Form::getValues()
{
    $values = array();
    foreach ($this->fields->all() as $name => $field) {
        if ($field->isDisabled()) {
            continue;
        }

fabpot added a commit that referenced this pull request Feb 26, 2016
@fabpot
bug #17542 ChoiceFormField of type "select" could be "disabled" (boul…
d0cac39 
…and)

This PR was submitted for the master branch but it was merged into the 2.3 branch instead (closes #17542).

Discussion
----------

ChoiceFormField of type "select" could be "disabled"

Hi,

New PR to add tests from the closed PR #15952

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets |
| License       | MIT
| Doc PR        |

Commits
-------

576c4b9 ChoiceFormField of type "select" could be "disabled"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
DomCrawler Status: Needs Work
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fabpot @xabbuh @stof @bouland @carsonbot
0