-
Notifications
You must be signed in to change notification settings - Fork 17
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 config to disable Requirements::customScript #200
base: 4
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## 4 #200 +/- ##
============================================
- Coverage 54.64% 54.06% -0.58%
- Complexity 80 81 +1
============================================
Files 5 5
Lines 280 283 +3
============================================
Hits 153 153
- Misses 127 130 +3
Continue to review full report at Codecov.
|
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.
Overall it works as expected, thanks! Just a couple updates noted.
Would it be helpful to list the arbitrary ID of the custom script include in the docs, in case someone wants to block / update (? if possible) the requirement in their code. |
I think that's a great idea. Now that we have it, and there are clear use cases, it's worth noting for easy reference. |
@@ -205,6 +205,11 @@ public function contentcontrollerInit() | |||
*/ | |||
public function getCustomScript() | |||
{ | |||
|
|||
if ($this->owner->config('clear_requirements')) { |
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.
Sorry, one other thing I just noticed. I believe this implementation will always return true
, as it will return the full config for $this->owner
. Updating to $this->owner->config()->get('clear_requirements')
would have the expected result.
Adding a configuration to disable injecting the custom JavaScript code into the pages. Also added a custom ID to the Requirements::customScript, as per best practices, to allow blocking the requirement programatically.
My use case: I am including the
silverstripe/elemental-flexslider
but am using completely my own templates and all the frontend code. I have no way to exclude the custom JS required with this module. I also think a configuration is a simpler step than clearing the requirement.