Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Update form choice type reference (refs #5179) #5847

Closed
wants to merge 6 commits into from

Conversation

althaus
Copy link
Contributor

@althaus althaus commented Oct 29, 2015

Q A
Doc fix? yes
New docs? yes
Applies to 2.7
Fixed tickets #5179

This PR aims to document the changes to the form choice type introduced in Symfony 2.7

@ureimers
Copy link

Perfect. I just found out, that https://github.com/symfony/doctrine-bridge/blob/2.7/Form/Type/DoctrineType.php#L135 actually makes use of the choice_loader option. This could be used as a reference when documenting (not in the documentation itself but as a working example of how to use the option).

A possible reference for "before/after" sections might be: https://gist.github.com/mickaelandrieu/5211d0047e7a6fbff925#form (the part about choice_list and following).


The ``choices_as_values`` option of ChoiceType was introduced in Symfony 2.7.

The ``choices_as_values`` option was introduced to ensure backward compatibility with the
Copy link
Member

Choose a reason for hiding this comment

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

The lines or this paragraph are a bit too long.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@javiereguiluz What's the official line length? Seems I'm blind as I cannot find anything in the contribution guide. :(

Copy link
Member

Choose a reason for hiding this comment

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

We usually break lines after the first word that crosses the 72nd character: http://symfony.com/doc/current/contributing/documentation/standards.html#sphinx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@xabbuh Thanks. Just pushed an update to fix this.

@weaverryan
Copy link
Member

Thanks @althaus for the start - it was actually very helpful! I've taken your commits and continued in #5876.

Cheers!

@weaverryan weaverryan closed this Nov 8, 2015
@althaus althaus deleted the improve_form_choice_reference branch November 10, 2015 08:11
@althaus
Copy link
Contributor Author

althaus commented Nov 10, 2015

@weaverryan Check!

weaverryan added a commit that referenced this pull request Nov 11, 2015
…weaverryan)

This PR was submitted for the 2.8 branch but it was merged into the 2.7 branch instead (closes #5876).

Discussion
----------

Symfony 2.7 Form choice option update

Hi guys!

This replaces #5755 #5847 and should close #5179 (or at least covers most things).

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | yes
| Applies to    | 2.7+
| Fixed tickets | #5179

This is a big change, and I rewrote a lot. If you have time to review, I think it's best to read the published version up platform.sh and check the code for validity. You can see the code in a project that inspired all the examples here: https://github.com/weaverryan/form_choices/tree/finished.

Commits
-------

b55d170 typo!
70f5e85 tweaks thanks to comments
0ad7b18 fixing wrong image name
69fc3c3 removing scaling on images
a2ad990 fixing bad reference
7aa4106 Completely re-working all the choice docs, including screenshots - for 2.7 changes
79c905c changing option order - choice_name and choice_value seem like edge cases
68725e8 Fixed preferred choices example
e2dea88 Fixed linebreaking
889f123 Added choices_as_values description [amend]
e1f583b Added choices_as_values description
d3cf004 Added choice_loader basics
3763f71 Added basic choice type changes
fd35e21 Fix docs related with choice form type refactor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants