Skip to content

[Chartjs] Rely on Stimulus values instead of custom attributes #177

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

Merged
merged 1 commit into from
Nov 30, 2021

Conversation

tgalopin
Copy link
Contributor

Q A
Bug fix? no
New feature? no
Tickets -
License MIT

In preparation of Symfony UX 2.0, this PR replaces the usage of custom attributes by Stimulus values. It also add Webpack Encore as an optional dependency to leverage the stimulus_controller helper it provides in the Twig extension.

Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

Looks very good!

@@ -38,9 +40,10 @@ public function load(array $configs, ContainerBuilder $container)
->setPublic(false)
;

if (class_exists(Environment::class)) {
if (class_exists(Environment::class) && class_exists(StimulusTwigExtension::class)) {
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@weaverryan weaverryan left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@weaverryan
Copy link
Member

Thank you Titouan!

@weaverryan weaverryan merged commit 8f9e8e7 into symfony:main Nov 30, 2021
@tgalopin tgalopin deleted the values-charts branch November 30, 2021 17:18
tgalopin added a commit that referenced this pull request Dec 7, 2021
…tributes (tgalopin, weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Cropperjs] Rely on Stimulus values instead of custom attributes

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Tickets       | -
| License       | MIT

Follows #177

Commits
-------

5ad7e32 Putting all Cropper.js options under cropper_options
e3327eb WIP
2f0df8b [Cropperjs] Rely on Stimulus values instead of custom attributes
symfony-splitter pushed a commit to symfony/ux-cropperjs that referenced this pull request Dec 7, 2021
…tributes (tgalopin, weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Cropperjs] Rely on Stimulus values instead of custom attributes

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Tickets       | -
| License       | MIT

Follows symfony/ux#177

Commits
-------

5ad7e32 Putting all Cropper.js options under cropper_options
e3327eb WIP
2f0df8b [Cropperjs] Rely on Stimulus values instead of custom attributes
fullstackdeveloperwebapp added a commit to fullstackdeveloperwebapp/ux that referenced this pull request Aug 1, 2023
…tributes (tgalopin, weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Cropperjs] Rely on Stimulus values instead of custom attributes

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| Tickets       | -
| License       | MIT

Follows symfony/ux#177

Commits
-------

5ad7e32 Putting all Cropper.js options under cropper_options
e3327eb WIP
2f0df8b [Cropperjs] Rely on Stimulus values instead of custom attributes
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.

2 participants