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

Common issues fixes & some style and translation updates #44

Closed
wants to merge 5 commits into from
Closed

Common issues fixes & some style and translation updates #44

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Apr 7, 2017

#43

Copy link
Member

@NoResponseMate NoResponseMate left a comment

Choose a reason for hiding this comment

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

Please fix/discuss the commented stuff, and besides that, great work :D

@@ -26,6 +26,7 @@ Simple CMS for Sylius.
new \Symfony\Cmf\Bundle\MenuBundle\CmfMenuBundle(),
new \Symfony\Cmf\Bundle\RoutingBundle\CmfRoutingBundle(),
new \Lakion\SyliusCmsBundle\LakionSyliusCmsBundle(),
new \Symfony\Cmf\Bundle\MediaBundle\CmfMediaBundle(),
Copy link
Member

Choose a reason for hiding this comment

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

Would be better to move it right above CmfMenuBundle, for consistency's sake, since everything else is apparently in alphabetical order. Of course if there's no inner conflict with the loading order of these bundles.

link: "Link"
name: "Name"
title: "Title"
body: Body
Copy link
Member

Choose a reason for hiding this comment

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

Quoting the messages is a precaution since some words are restricted in yaml, etc. yes/no get transformed to boolean when not encapsulated in quotes.
Possibly there're more words like that, and it may cause unexpected visual issues further on.

routes: Routes
static_contents: Static contents
string_blocks: String blocks
taxon_blocks: Taxon blocks
Copy link
Member

Choose a reason for hiding this comment

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

Missing new line at the end.

8. Initialize PHPCR repository

```bash
$ bin/console doctrine:phpcr:repository:init
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -31,7 +31,7 @@ public function buildForm(FormBuilderInterface $builder, array $options)
->add('image', FileType::class, [
'label' => 'lakion_sylius_cms.form.taxon_block.image',
])
->add('taxon', TaxonChoiceType::class, [
->add('taxon', TaxonAutocompleteChoiceType::class, [
Copy link
Member

Choose a reason for hiding this comment

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

Quite sure you'd also need to update the template to make it work as expected.
Just add {% form_theme form '@SyliusAdmin/Form/theme.html.twig' %} at the top of views\Admin\TaxonBlock\_form.html.twig

@michalmarcinkowski
Copy link
Member

Merged in #48. Thanks for your work Mikołaj! 👍

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.

3 participants