-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Added a "Welcome Page" for new Symfony apps #834
Conversation
{% endblock %} | ||
|
||
{% block stylesheets %} | ||
<style type="text/css"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type attribute is redundant and not required
I don't think that writing the whole logic of the welcome page directly in the We removed the entire |
*/ | ||
public function indexAction() | ||
{ | ||
return $this->render('default/index.html.twig'); | ||
require_once __DIR__.'/../../../app/SymfonyRequirements.php'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a comment about this, something like:
// replace this example code with whatever you need
@Pierstoval The logic is just a couple of lines of code - that's quite easy to remove. 👍 @javiereguiluz is already aware of this, but we'll need a docs PR when this is merged - it requires some screenshots, so there's no use doing it before a merge. |
I've done the suggested changes. Given that the current situation is dreadful (the first thing you see about Symfony is an error page) I ask you to please make a decision about this PR as quickly as possible. Thanks! |
👍 |
Great improvement! 👍 |
👍 |
@@ -12,6 +12,7 @@ public function testIndex() | |||
|
|||
$crawler = $client->request('GET', '/'); | |||
|
|||
$this->assertTrue($crawler->filter('html:contains("Homepage")')->count() > 0); | |||
$this->assertEquals(200, $client->getResponse()->getStatusCode()); | |||
$this->assertContains('Welcome to Symfony', $client->getResponse()->getContent()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest keeping a crawler-based example too in the test (or remove the unused $crawler
variable)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point! I've changed it by:
$this->assertContains('Welcome to Symfony', $crawler->filter('#container h1')->text());
Thanks.
👍 |
1 similar comment
👍 |
👍 |
</svg> | ||
|
||
Read Symfony documentation to learn | ||
<a href="http://symfony.com/doc/{{ symfony_version }}/book/page_creation.html">How to create your first page in Symfony</a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why not just use constant('\\Symfony\\Component\\HttpKernel\\Kernel::VERSION')
here as well? Symfony.com is capable of handling x.y.z versions in the URL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the tip. I've simplified the template. Thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just thought of a case that is no longer supported: unstable versions (where the version will be 2.8-dev
for instance of 2.8-rc1
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, you shouldn't use a leading slash in the class name. Class names are defined without that slash.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GrahamCampbell removed the leading slash. Thanks!
@wouterj what about making this check for unstable versions?
{{ symfony_version|length > 3 ? 'current' : symfony_version }}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javiereguiluz just thought of a nicer solution: {{ symfony_version[:3] }}
. If the version is 2.7.3
, it'll link to /doc/2.7/book/...
and when the version is 3.0-dev
, it'll link to /doc/3.0/book/...
, which is redirected to /doc/master/book/...
.
Btw, can you please add the 2.8 version to the documentation?
This is a simple but very effective improvement in my opinion. Great idea @javiereguiluz |
I think we're trying to mix two different concepts here:
When I use the standard edition, I don't directly open a page in the browser: I first start writing my app and then I'll check it later. On the other hand, I do think we're missing a good tool to check if the environment is set up correctly. But I'd rather see it as a standalone script you could run everywhere. |
For people using the Symfony Installer, all technical requirements are checked during the installation and you can see the list of things to fix. In case you ignore this list, the first thing you'll see in the browser is the message telling you that some requirements aren't met. For people still using Composer to install Symfony, they don't get the nice requirements checker of the installer, so it's nice to see the welcome page reminding them about these errors. For Symfony experts, the new welcome page is just a 3 lines controller and 1 Twig template. They will clean everything very easily. |
Contrary to the common denominator I am 👎 because this creates confusion, maybe even documented confusion. I would have initially thought that a default route on / is more than enough. |
@cordoval I don't understand your comment 😕 This PR is precisely to provide a default route to |
Instead of the demo doing a /examples/page, it could just be /. The demo in the first place missed the point completely. |
My feeling is broader actually, I remember those days in which symfony was simple 😊 and that we were happy escaping the magic of sf14 and give freedom to users. I know it is a hard critic, I did one project in sf14 for an old client recently and it felt to me simpler than working with symfony2. That feeling sank in me heavily. |
I have the feeling that your comments are completely not related to this PR, @cordoval. Can you maybe create a new issue for your thoughts on this (as it can lead to interesting results, when thinking about what can be made easier?) in the symfony repository and avoid the noise here? Thanks! 😃 |
@cordoval I don't understand why things are "more complicated" with this PR, even if I'm 👎 for the way it was implemented. For the rest, I still think that, even if its "only 3 lines of code and a template", it's still much more than "one single line of code" for me... |
👍 for the current state and merging this one quickly |
@javiereguiluz Can you update the screenshot in the description to avoid comments about the previous version? |
Done. |
👍 |
1 similar comment
👍 |
It's not magical or anything (like that it's part of the FrameworkBundle defining a default route), its a sensible default with a single page (not a complete bundle). And you are going to change (rather then delete - aka more work) these files anyway, so 👍 for me.
Never used Symfony 1.4 :) so can't really tell, what is simple is not automatically easy. |
@@ -4,15 +4,17 @@ | |||
|
|||
use Sensio\Bundle\FrameworkExtraBundle\Configuration\Route; | |||
use Symfony\Bundle\FrameworkBundle\Controller\Controller; | |||
use Symfony\Component\HttpFoundation\Request; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided at some point to keep such a use statement to make it easier for people to start working right away. Not sure if we need to keep it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we keep it, it should at least be in the indexAction
method to show why its here.
I wouldn't use the installer nor composer bootstrap when creating a new server for an existing sf app. I'm actually longing for a standalone script that could be used anywhere, but that's another topic, sorry for bringing it here. Now back to the actual PR: I don't get the point of having a default page when all I want is an empty skeleton. |
@gnugat I helped a user on Twitter just last week - they got the 404 page and thought that tons of things must be broken with their setup because they got an error page when first loading things up. That's the point. |
@weaverryan in the case of developers trying out Symfony for the first time, doesn't it make sense to provide them a different application (let's say the AcmeDemo show case application)? To me it sounds more like a lack of explanation / choice on the download page. |
Guys, we are trying to help the newcomers, if you already know Symfony, you will know that you need to delete 3 lines of code and one twig template, and that won't take much time even if you have to do it multiple times. For a newcomer this change can make a big difference. |
@gnugat It depends of what kind of "newcomer" you are. In my case, when I want to discover new frameworks/languages, I won't look at the code at first. I'll create an app. So in the case of Symfony, if I had to discover it today, I'd create an app and start to code right now by following the docs/books. Even if the Symfony Demo app is great in many points, it depends on each person, and IMO is more for presentations and learnings. And many other developers will give you different answers. That's why providing a very small working skeleton is absolutely important. |
Thank you @javiereguiluz. |
This PR was squashed before being merged into the 2.3 branch (closes #834). Discussion ---------- Added a "Welcome Page" for new Symfony apps ### Problem When you create a new Symfony app (`symfony new my_project`) and follow the installer instructions, when browsing the application for the first time, you'll get a 404 error: ![old_welcome_screen](https://cloud.githubusercontent.com/assets/73419/8820296/760f30fc-3054-11e5-9251-d382a5257758.png) ### Solution In this PR we add a new simple welcome page to avoid that error. This page is designed for newcomers, so we'll guide them exactly to where they should go: to the doc explaining how to create their first page with Symfony: ![new_welcome_screen_ok](https://cloud.githubusercontent.com/assets/73419/8820314/97ee4258-3054-11e5-8ef8-f4f8c93bb899.png) Commits ------- 0e4bb3e Added a "Welcome Page" for new Symfony apps
Problem
When you create a new Symfony app (
symfony new my_project
) and follow the installer instructions, when browsing the application for the first time, you'll get a 404 error:Solution
In this PR we add a new simple welcome page to avoid that error. This page is designed for newcomers, so we'll guide them exactly to where they should go: to the doc explaining how to create their first page with Symfony: