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

Backport deprecation changes for Bootstrap 2 #1158

Open
wants to merge 10 commits into
base: v2.3.x
Choose a base branch
from

Conversation

carlo1138
Copy link

@carlo1138 carlo1138 commented Feb 15, 2016

Similar to #1115 but with support for Symfony 2.3

  • Ported test suite and adapted to Bootstrap 2
  • Ported deprecation changes from master
  • Configured travis

What do you think?

@tarjei
Copy link

tarjei commented Apr 7, 2016

Hi, I would love to see movement on this one!

@carlo1138
Copy link
Author

carlo1138 commented May 31, 2016

My plan was to add more tests, but I don't have much time now (I've already added some). Anyway I'm using my fork in a Symfony 2.7 project + Bootstrap 2, so I would like to have a feedback on this PR

@phiamo
Copy link
Owner

phiamo commented May 31, 2016

sorry did not see this at all :(

does the testsuite run with symfony3 ? i remember there were some issues.

@carlo1138
Copy link
Author

does the testsuite run with symfony3 ? i remember there were some issues.

Now it's passing, but I haven't ported all the tests form master, some were not compatible.

@trsteel88
Copy link
Contributor

What's left to do here? Would love to start using this on 2.x

@phiamo
Copy link
Owner

phiamo commented Jun 8, 2016

can we review this thogether: https://travis-ci.org/phiamo/MopaBootstrapBundle/builds/136118152

@trsteel88
Copy link
Contributor

I can't seem to find @carlo1138 branch to make changes. I've had to checkout the actual commit.

All of the config yml files should wrap parameters with quotes. From 4.0 exceptions will be thrown.

Also, if you actually run this version, it fails. The extensions are wrong:

    mopa.form.help_extension:
           class: Mopa\Bundle\BootstrapBundle\Form\Extension\HelpFormTypeExtension
           arguments:
                - { tooltip_icon: %mopa_bootstrap.form.tooltip.icon%, tooltip_placement: %mopa_bootstrap.form.tooltip.placement% }
           tags:
                - { name: form.type_extension, alias: form }

Should be

  • { name: form.type_extension, _extended_type: Symfony\Component\Form\Extension\Core\Type\FormType_ }

Also, some Types haven't been updated. e.g. TabType still has "public function setDefaultOptions(OptionsResolverInterface $resolver)"

@carlo1138
Copy link
Author

All of the config yml files should wrap parameters with quotes. From 4.0 exceptions will be thrown.

Do we need to support 4.0? 😄

Should be
{ name: form.type_extension, extended_type: Symfony\Component\Form\Extension\Core\Type\FormType }

In which version?

Also, some Types haven't been updated. e.g. TabType still has "public function setDefaultOptions(OptionsResolverInterface $resolver)"

Yep, if we want both 2.x and 3.x support, we need to use both methods, like it was suggested in #1115 (comment)

Now I've rebased my branch with a new travis.yml, it's not the same as your v2.3.x, but similar
https://travis-ci.org/carlo1138/MopaBootstrapBundle/builds/136356781
I've removed symfony < 2.3.x, and excluded php < 5.5 for symfony > 2.8

@carlo1138
Copy link
Author

@trsteel88

Should be
{ name: form.type_extension, extended_type: Symfony\Component\Form\Extension\Core\Type\FormType }

In which version?

Also, some Types haven't been updated. e.g. TabType still has "public function setDefaultOptions(OptionsResolverInterface $resolver)"

Yep, if we want both 2.x and 3.x support, we need to use both methods, like it was suggested in #1115 (comment)

LOL, I had already done both of them, I've just forgot about it, see f6a4a12. There is a BC layer to support all the symfony versions.

@trsteel88
Copy link
Contributor

Form aliases are deprecated and will be removed in 3.0. You must use the Fully Qualified Class Names instead of form aliases now.

@carlo1138
Copy link
Author

@trsteel88

Some types seem to still be missing the bc. See
...
Form aliases are deprecated and will be removed in 3.0. You must use the Fully Qualified Class Names instead of form aliases now.

Done

@phiamo
I'm trying to add a test for tabs, but they don't render properly, it seems like you deleted the rendering functions in d7753ff from fields.html.twig. Can you help me understand what's wrong?

@phiamo
Copy link
Owner

phiamo commented Feb 3, 2017

@carlo1138 what is the status here?

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.

4 participants