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

[Waiting Code Merge] Introduction chapter for the Form component #1883

Merged
merged 7 commits into from
Sep 9, 2013

Conversation

webmozart
Copy link
Contributor

This PR is WIP. It contains the sketch for an introduction chapter of a new Form section in the components book. This section is dedicated to:

  • explain how to use the Form component without Symfony2
  • document the Form component in detail (also for framework users)

I want to achieve both aspects at once by introducing two new configuration blocks:

  • "Standalone PHP" (php-standalone) for PHP code that does not rely on the full-stack framework
  • "With Symfony" (php-symfony) for PHP code that assumes the usage of the full-stack framework

The differences between code in these two formats is in most cases minimal.

@fabpot: What do you think about this? Could you adapt your Sphinx extension accordingly?

@weaverryan: Do you want to write the accompanying text to the code samples? (where missing)

everyone: Is anything essential missing in this chapter?

->getFormFactory();

TODO: Cross reference the HTTP foundation chapter where the configuration of the
Templating component is explained.
Copy link
Member

Choose a reason for hiding this comment

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

This sentence looks weird

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it's a TODO note anyway... ;)

Copy link
Contributor

Choose a reason for hiding this comment

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

why it looks weird @stof ?

Copy link
Member

Choose a reason for hiding this comment

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

HTTP foundation chapter vs Templating component is explained

@richardmiller-zz
Copy link
Contributor

So far the differences between standalone and within the framework usage have been dealt with by documenting the standalone way in the component section and then linking to cookbook pages which show differences in framework specific usages.

Whether this is the approach here or you have you the framework specific code blocks, there is still a general issue with people not realising that the components section contains details relevant even if you are using the framework.

@webmozart
Copy link
Contributor Author

@richardmiller Yes. My idea is to make the form chapter in the book more or less the equivalent of this introduction chapter (only without the component-specific parts) and to link from there to the other chapters of the component docs. I.e., the component chapter "Introduction" and the book chapter "Forms" will give info about how to get started, and the other chapters in the component docs will give more information for both framework and non-framework users.

What do you think about this?

@richardmiller-zz
Copy link
Contributor

Sounds good, would be in keeping with other component sections where the more detailed documentation is now in the component section. Some of the cookbook pages can possibly moved to the component section with just framework specific parts being kept in the cookbook.

If the new blocks are introduced then it will remove the need for the cookbook pages specifying the differences especially where they are minor. It does mean a change in thinking about keeping framework specific info out of the components section though.

@webmozart
Copy link
Contributor Author

I guess we need to find out whether this works anyway. Some more ideas on how to make this work:

  • also put explanations into configuration blocks where they differ between framework vs. non-framework
  • when switching tabs of a configuration block, switch all tabs of the same type on the page and remember it in a cookie (in other words, if I select "With Symfony" once, all code samples will show the "With Symfony" tab)
  • when downloading the PDF, let the user select which configuration blocks the PDF should include (e.g. "With Symfony", "YAML")

ping @fabpot

@lyrixx
Copy link
Member

lyrixx commented Nov 4, 2012

I am really +1 with the cookie.
But I am -1 with the doc as PDF. Since you can not switch in the PDF, the PDF should contain all section.

@weaverryan
Copy link
Member

Hi guys!

Using the tabbed code blocks to handle framework vs standalone docs is much different than what we do now. But, we're always in the process of trying, refining and improving. My first impression is that these blocks are a bit more clear than I expected, and since we don't have any form component docs right now, I think this is the right place to try it.

Making the changes needed to the sphinx-php is trivial (2 lines in there, then Fabien will need to add 2 more lines to the main conf.py file). I'll make the pull request to add it. We do need to name the tags - my initial thoughts were Standalone Use and Framework Use - but I'm quite open to other suggestions. It should be somewhat short, but fully explanatory.

Here are things that will take more work (which would need to be done by @fabpot). Since we're experimenting still, I don't think these things need to happen immediately anyways:

  • The idea of having all tabs change when one changes & using a cookie. We'll know more about how important this is and if we want it to work differently anyways after actually seeing the docs up and rendered
  • Modifying PDF renders. Currently, the first "tab" will render into the PDF only. We would probably need to do something more intelligent (I like @lyrixx's idea), but it does not need to happen now.

@bschussek I'll see if I can take your document and work it over (proofread it, add more content where needed etc) by this weekend - then we should have something we could merge in :).

@fabpot If you disagree or have any major concerns with trying to put the framework versus component code into configuration-block tabs where possible, let us know! I still think that if the differences are major, it would still make sense to have a whole separate doc (i.e. cookbook entry) to explain the framework use. And of course, when we read the docs, we need to read from 2 perspectives (component user & framework user), and make sure that the experience still feels continuous and complete.

Thanks everyone!

@fabpot
Copy link
Member

fabpot commented Nov 8, 2012

For the PDF, and even if this is more work, I prefer @bschussek idea, where the user selects which tab he wants to have in the book. I don't like the idea of putting all sections in the PDF as it will become a mess pretty fast with pages after pages of code.

But then, what about the printed book? We don't have one yet, but as people ask for it, we will probably have to publish a paper book at some point.

@fabpot
Copy link
Member

fabpot commented Nov 8, 2012

@weaverryan No problem for the adding of new possible sections under the configuration-block.

@webmozart
Copy link
Contributor Author

The idea of having all tabs change when one changes

If we do this we have to find out how to deal with tabs of different length. I.e., if I switch the tab of conf-block 3 on a page, and blocks 1 and 2 switch automatically but to a tab of a different length, the view will "jump".

But then, what about the printed book? We don't have one yet, but as people ask for it, we will probably have to publish a paper book at some point.

I think the most viable option is to choose one combination for the book and point out in the introduction that the user may find the code samples of the alternatives online.

@webmozart
Copy link
Contributor Author

@weaverryan Thanks, very appreciated!!

->getFormFactory();

To secure your application against CSRF attacks, you need to define a CSRF
secret. Generate a random string with at least 32 characters, insert it in the
Copy link
Contributor

Choose a reason for hiding this comment

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

secret number or string parameter, although it is obvious but more proper


The Form component comes with tight (but optional) integration with Symfony's
Validator component. If you're using a different solution for validation,
no problem! Simply take the bound data of your form (which is an array or
Copy link
Contributor Author

Choose a reason for hiding this comment

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

"bound" -> "submitted". In this chapter, people don't really know yet what "binding" refers to.

@webmozart
Copy link
Contributor Author

@weaverryan I added my comments inline. As for your questions:

What about the pure PHP templating engine integration that uses the Form component? Is this integration possible?

Yes, but I'd like to defer the detailed explanation for this to the "Templating" or "Rendering" chapter of the component docs. Once we have this, we can add a note here.

The validation constraints and form types reference need to be separated somehow so when we link to them, you know which are framework-only (or at least require more bridge integration)

Yes, I think the references in general need to be improved. Should we create a list with the todos we're identifying here and store it somewhere?

I feel pretty good overall. Not so good that we should be in a rush to take apart the form book chapter

We can decide what to do with the form chapter when we finished the component docs. Then we'll see whether this whole plan works out or not. :)

In general I like your additions very much! I think they are very clear and well formulated! (and much better than I could have done) Thanks Ryan!!

I'll reread this tonight or tomorrow and try to give some more feedback.

public function newAction(Request $request)
{
// createFormBuilder is a shortcut to get the "form factory"
// and then call "createBuilder" on it
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this should be "createBuilder()". Names of methods in the text of the documentation also always include the parentheses.

@weaverryan
Copy link
Member

Hi @bschussek!

I've made the changes you suggested and fixed a few other things! I've also made a PR to fabpot/sphinx-php#3 so that we have the new code blocks that we'll need. After that PR is merged, we can merge this in and try it out.

Sorry for the delay on my end. What are the next steps from here?

Thanks!

@weaverryan
Copy link
Member

Btw, we're waiting for the code merge of fabpot/sphinx-php#3 and then we'll merge this in.

@wouterj
Copy link
Member

wouterj commented Feb 2, 2013

what is the status of this @fabpot

@wouterj
Copy link
Member

wouterj commented Apr 17, 2013

@fabpot and @weaverryan, is there any progress on this one? I'd like to move this forward, so we can create more form component articles (e.g. the empty_data article written by Joel and the form_collections article I'm going to write)

@wouterj
Copy link
Member

wouterj commented May 20, 2013

What about removing the special code blocks from this PR (only use php-standalone) and putting that special blocks inside a new PR? IMO, this is one of the most wanted missing docs and it's stupid (note: this is not negative meaned, I just couldn't find the correct word) to not merge such it because of some experimental thing.

@weaverryan @bschussek

@weaverryan
Copy link
Member

+1 I think that's a very pragmatic idea. @bschussek if you have time and agree, do it! Otherwise, I'll handle it shortly.

@wouterj
Copy link
Member

wouterj commented Jun 30, 2013

ping @bschussek @weaverryan

@wouterj
Copy link
Member

wouterj commented Jul 31, 2013

@bschussek you're now back, so my last ping 🌕 ;-)

@webmozart
Copy link
Contributor Author

I'd prefer to get fabpot/sphinx-php#3 merged, otherwise we'll remove the duplicate blocks now and add them in again later - too much unnecessary work for my taste.

@mattattui
Copy link
Contributor

fabpot/sphinx-php#3 has just been merged!

@webmozart
Copy link
Contributor Author

@wouterj This should be ready to merge now :)

@wouterj
Copy link
Member

wouterj commented Sep 3, 2013

yeah, yeeh! 🎈

ping @weaverryan

weaverryan added a commit that referenced this pull request Sep 9, 2013
[Waiting Code Merge] Introduction chapter for the Form component
@weaverryan weaverryan merged commit 9f07fc4 into symfony:2.1 Sep 9, 2013
@weaverryan
Copy link
Member

Woot! Merged! Now we'll see how this look and continue to experiment.

Cheers!

@ggam
Copy link

ggam commented Sep 10, 2013

Just a question, shouldn't the code examples use handleRequest() instead of bind()?

@xabbuh
Copy link
Member

xabbuh commented Sep 10, 2013

handleRequest() has been introduced in Symfony 2.3. This pull requests was based on Symfony 2.1. So, this is correct. But you can create a pull request (if there isn't already one) to change the examples for Symfony 2.3.

@webmozart
Copy link
Contributor Author

@ggam You are correct. The chapter needs to be updated for versions >2.1. If you have time, please open a PR! :)

@xabbuh
Copy link
Member

xabbuh commented Sep 13, 2013

I made the necessary changes for handleRequest() in #2974. Has anything else changed in the Form component that needs to be reflected?

@weaverryan
Copy link
Member

@xabbuh thanks for doing that! Also, bind became submit. And there is new handling for adding buttons and configuring the URL and method of the form. But, bind -> submit is the most important.

I've updated your #2969 issue with a checklist to make sure we have everything.

Thanks!

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.