Skip to content

[WIP] moving to Symfony2.1 and using the CMF for content rendering #6

Merged
merged 22 commits into from Sep 26, 2012

3 participants

@lsmith77
Symfony CMF member

@adou600 @mjanser @leahaense great work

I have created the PR so that we can coordinate the finalization of the changes.
One thing I just noticed is that the stuff in app/tests should be updated to the new fixtures.
Also the README.md should be adjusted.

I havent tested it all, but from what I saw it looks like you guys did an awesome job.

@dbu
Symfony CMF member
dbu commented Sep 10, 2012

seems i can't comment on the file mode change, but why make .htaccess executable? that one makes no sense.

@dbu dbu and 1 other commented on an outdated diff Sep 10, 2012
src/Cmf/MainBundle/Menu/CustomRenderer.php
+ * This file is part of the Cmf\MainBundle
+ *
+ * (c) Lukas Kahwe Smith <smith@pooteeweet.org>
+ *
+ * This source file is subject to the MIT license that is bundled
+ * with this source code in the file LICENSE.
+ */
+
+namespace Cmf\MainBundle\Menu;
+
+use \Knp\Menu\Renderer\TwigRenderer;
+use \Knp\Menu\ItemInterface;
+use Knp\Menu\MenuFactory;
+use \Knp\Menu\MenuItem;
+
+class CustomRenderer extends TwigRenderer
@dbu
Symfony CMF member
dbu added a note Sep 10, 2012

should we not rather create a custom menu provider than a renderer? this has nothing to do with rendering...

@adou600
Symfony CMF member
adou600 added a note Sep 21, 2012

Works for me. However, I've never played with a menu provider before. Do you have any example somewhere, which works with the latest version of Symfony and the CMF?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu
Symfony CMF member
dbu commented Sep 10, 2012

there is also other files that go to 755 instead of 644 that should be changed back.

and one more question: why remove the compiled slidesets from web/slides/* ? afaik we do not provide them anywhere else and they are useful introduction.

@adou600
Symfony CMF member
adou600 commented Sep 21, 2012

I added the compiled slides in the web folder, the tests, readme and homepage have been updated. About the file permissions (755 instead of 644), can I do something about it? I'm not sure to get the problem...

I'm now waiting for instructions about the menu renderer...

@adou600 adou600 was assigned Sep 21, 2012
@adou600
Symfony CMF member
adou600 commented Sep 26, 2012

I think the work is done. Tell me if you see anything that could be improved.
I'm just not sure how the site will behave on production, as I used it only with app_dev.php

@lsmith77 lsmith77 and 2 others commented on an outdated diff Sep 26, 2012
@@ -1,6 +1,14 @@
-/app/logs/*
-/app/cache/*
-/app/config/parameters.ini
-/vendor/
-/web/bundles/
-/app/bootstrap.php.cache
+composer.phar
+web/bundles/
+web/js/
@lsmith77
Symfony CMF member
lsmith77 added a note Sep 26, 2012

why do you ignore web/js and web/css?

@adou600
Symfony CMF member
adou600 added a note Sep 26, 2012

because I thought it should be installed with app/console assets:install. but when thinking about it it would mean that someone cloning the repository should type that command before using the site. I will remove it.

@lsmith77
Symfony CMF member
lsmith77 added a note Sep 26, 2012
@dbu
Symfony CMF member
dbu added a note Sep 26, 2012

just for completeness: if we would use assetic, you would also want to ignore web/assets/ because assetic:dump renders js and css into that folder.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@lsmith77 lsmith77 commented on an outdated diff Sep 26, 2012
web/reload-fixtures.php
@@ -0,0 +1,16 @@
@lsmith77
Symfony CMF member
lsmith77 added a note Sep 26, 2012

please remove this file .. it was only meant for the sandbox to reset the fixtures in case someone broke things via the admintool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu dbu commented on the diff Sep 26, 2012
src/Cmf/MainBundle/Menu/MenuProvider.php
+use Knp\Menu\FactoryInterface;
+use \Knp\Menu\MenuItem;
+use Symfony\Component\DependencyInjection\ContainerAware;
+use Symfony\Component\HttpFoundation\Request;
+
+class MenuProvider extends \Symfony\Cmf\Bundle\MenuBundle\Provider\PHPCRMenuProvider
+{
+ public function get($name, array $options = array())
+ {
+ if ('simple' == $name)
+ {
+ //get the internal pages of the website
+ $menu = parent::get($name, $options);
+
+ //Home menu item
+ $item = new MenuItem('Home', $this->factory);
@dbu
Symfony CMF member
dbu added a note Sep 26, 2012

imho we can merge this, but:

@lsmith77 what do you think about this one? we could also have handled this like an "external" link but that sounds stupid and would be hardcoding information. eventually we should probably do something on the cmf MenuBundle to allow rendering the root of the menu as home link.

should we create a ticket on MenuBundle so we don't forget?

@lsmith77
Symfony CMF member
lsmith77 added a note Sep 26, 2012

sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@dbu
Symfony CMF member
dbu commented Sep 26, 2012

looks good to me, thanks everybody who worked on this!

can we deploy like this on cmf.symfony.com or do we need to talk with somebody?

now the next step would be to add CreateBundle and do some editing :-) but then we need access protection...

@lsmith77
Symfony CMF member

i can deploy it ..

@dbu dbu referenced this pull request in symfony-cmf/menu-bundle Sep 26, 2012
Closed

Home link #15

@lsmith77
Symfony CMF member

merged

@lsmith77 lsmith77 merged commit f853036 into master Sep 26, 2012

1 check passed

Details default The Travis build passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.