-
-
Notifications
You must be signed in to change notification settings - Fork 364
[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
Conversation
cb0b182
to
2d9ad7c
Compare
There was a problem hiding this 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)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using willBeAvailable()
would be nice, but it's not available until 5.3 https://github.com/symfony/symfony/blob/e9a702635a89e84d2c63f25d066e63d729017741/src/Symfony/Component/DependencyInjection/ContainerBuilder.php#L1460
2d9ad7c
to
ebeaedb
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good 👍
Thank you Titouan! |
…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
…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
…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
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.