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

Add webpack and missing datepicker scripts #329

Merged
merged 7 commits into from
May 8, 2022

Conversation

jordisala1991
Copy link
Member

@jordisala1991 jordisala1991 commented May 6, 2022

Subject

I am targeting this branch, because this is BC.

Changelog

### Added
- Add webpack configuration to compile the date picker scripts used by the form types.

package.json Outdated Show resolved Hide resolved
@jordisala1991
Copy link
Member Author

Companion PR for SonataAdminBundle: sonata-project/SonataAdminBundle#7809

@jordisala1991
Copy link
Member Author

jordisala1991 commented May 7, 2022

So, with the current config, everything works, jQuery is completely shared across SonataAdmin and form extensions, and we moved everything related to date picker here.

There is only one part that is still on SonataAdminBundle:

            {# localize moment #}
            {% set localeForMoment = canonicalize_locale_for_moment() %}
            {% if localeForMoment %}
                <script src="{{ asset('bundles/sonataform/moment-locale/' ~ localeForMoment ~ '.js') }}"></script>
            {% endif %}

When we update the date time picker library, this won't be necessary. what should we do about it?

@jordisala1991
Copy link
Member Author

Another thing that we should ensure, is that this move does not require a new major of AdminBundle, to do so, we might add this PR to 1.x instead.

@VincentLanglet
Copy link
Member

Another thing that we should ensure, is that this move does not require a new major of AdminBundle, to do so, we might add this PR to 1.x instead.

Oh I misunderstood sonata-project/SonataAdminBundle#7809,
You added support for 2.x but lost support for 1.x since the js was not provided.

We have two solutions

  • Keeping the js in SonataAdmin to have support for 1.x, and loading the 'bundles/sonataform/app.js' file if it exists to have the support for 2.x
  • Moving this PR to 1.x, and adding a conflict in SonataAdmin.

I don't think this PR is a BC break, so we might do it in 1.x indeed ; this would be better.

@jordisala1991 jordisala1991 force-pushed the feature/add-webpack branch from b6dc581 to c0792f1 Compare May 8, 2022 07:17
@jordisala1991 jordisala1991 changed the base branch from 2.x to 1.x May 8, 2022 07:17
@jordisala1991 jordisala1991 marked this pull request as ready for review May 8, 2022 07:17
@jordisala1991
Copy link
Member Author

@jordisala1991
Copy link
Member Author

Another thing that we should ensure, is that this move does not require a new major of AdminBundle, to do so, we might add this PR to 1.x instead.

Oh I misunderstood sonata-project/SonataAdminBundle#7809, You added support for 2.x but lost support for 1.x since the js was not provided.

We have two solutions

  • Keeping the js in SonataAdmin to have support for 1.x, and loading the 'bundles/sonataform/app.js' file if it exists to have the support for 2.x
  • Moving this PR to 1.x, and adding a conflict in SonataAdmin.

I don't think this PR is a BC break, so we might do it in 1.x indeed ; this would be better.

I think adding it to 1.x make more sense, rebased.

@jordisala1991 jordisala1991 merged commit 46f5401 into sonata-project:1.x May 8, 2022
@jordisala1991
Copy link
Member Author

This one will require a minor release, and then we can work on the integration with the admin bundle

@jordisala1991 jordisala1991 deleted the feature/add-webpack branch May 10, 2022 07:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants