Skip to content

Added "data" option to the "Inherited options" table. #3333

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

Merged
merged 1 commit into from
Dec 26, 2013

Conversation

mtrojanowski
Copy link
Contributor

Q A
Doc fix? yes
New docs? no
Applies to 2.3+
Fixed tickets #2871

@wouterj
Copy link
Member

wouterj commented Dec 14, 2013

👍

@@ -21,7 +21,8 @@ forms, which is useful when creating forms that expose one-to-many relationships
| | - `prototype_name`_ |
+-------------+-----------------------------------------------------------------------------+
| Inherited | - `label`_ |
| options | - `error_bubbling`_ |
| options | - `data`_ |
| | - `error_bubbling`_ |
Copy link
Member

Choose a reason for hiding this comment

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

Imho it is rather confusing that the order of the options in the table differs from the order below. What do you guys think? This is, of course, not exclusively related to this PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The entries are anchors anyway so I don't think it's that of a problem.

Copy link
Member

Choose a reason for hiding this comment

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

I prefer to put them in the order of the file (we did that everywhere currently)

@mtrojanowski
Copy link
Contributor Author

Updated the changes based on previous comments and discussion in #2871:

  • fixed the order of the items in tables
  • added a note to the data partial
  • Changed a note in the checkbox type to be more clear about how to set the default


The default values for Form fields are taken directly from the
Copy link
Member

Choose a reason for hiding this comment

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

I prefer form fields

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the next commit. Once we agree that everything is fine I can squash the commits.

@wouterj
Copy link
Member

wouterj commented Dec 18, 2013

👍 (which means: I approve this PR 😉)

Fixed sorting of options.
Made the note on selecting the checkbox by default more clear.
Added note on defaults to the `data` option partial.
@mtrojanowski
Copy link
Contributor Author

Rebased to latest 2.3. and squashed.

@xabbuh
Copy link
Member

xabbuh commented Dec 20, 2013

👍

weaverryan added a commit that referenced this pull request Dec 26, 2013
Added "data" option to the "Inherited options" table.
@weaverryan weaverryan merged commit 08d95b2 into symfony:2.3 Dec 26, 2013
@weaverryan
Copy link
Member

Wow, this is really great! Lot's of very precise changes, correct branch, squashes - you're welcome back here any time Michał ;). And don't be a stranger, Leanna and I very much liked hanging out with you!

Cheers and Thanks!

@mtrojanowski
Copy link
Contributor Author

I did promise you two more PR's in the coming year so I will become a much more frequent guest here.

I had a really nice time with you guys too (and it's really cool to see my name written with the Polish letters! :) )

Will contribute soon :)

@cordoval
Copy link
Contributor

👍 nice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants