-
-
Notifications
You must be signed in to change notification settings - Fork 6.9k
Allow optional jQuery in Validators and Widgets. #20528
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
base: 22.0
Are you sure you want to change the base?
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 22.0 #20528 +/- ##
============================================
+ Coverage 65.42% 66.74% +1.31%
- Complexity 11266 11372 +106
============================================
Files 428 445 +17
Lines 37064 37364 +300
============================================
+ Hits 24250 24937 +687
+ Misses 12814 12427 -387 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ield`, `ActiveForm`, fix customLabel for `checkbox` and `radio` methods.
…migration directories.
…implify `getClientOptions()` method.
…asset manager hash callback assignments.
…JqueryClientScript`.
…ptTest` and `ActiveFormTest` for clarity.
…ryClientScriptTest` for stricter comparison.
…ooleanValidatorJqueryClientScriptTest` for clarity.
…JqueryClientScript`.
…ryClientScript`.
… message assertions.
…age assertions and improve clarity in validation checks.
…interface implementation.
* | ||
* @since 2.2.0 | ||
*/ | ||
public bool $useJquery = false; |
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.
I don't think it is needed. Anything breaks if it's removed?
* | ||
* @since 2.2.0 | ||
*/ | ||
public bool $useJquery = true; |
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.
Should it be for web application only?
* @return string | ||
*/ | ||
protected function formatMessage($message, $params) | ||
public function formatMessage($message, $params) |
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.
Why changing it to public?
* @since 2.0.11 | ||
*/ | ||
protected function isClientValidationEnabled() | ||
public function isClientValidationEnabled() |
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.
Why is it public now?
* @since 2.0.11 | ||
*/ | ||
protected function isAjaxValidationEnabled() | ||
public function isAjaxValidationEnabled() |
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.
Why is it public now?
* @since 2.0.7 | ||
*/ | ||
protected function getInputId() | ||
public function getInputId() |
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.
Why is it public now?
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.
Pull Request Overview
This PR introduces optional jQuery functionality for Yii2 validators and widgets, allowing developers to disable jQuery while maintaining backward compatibility. The implementation adds a useJquery
property to yii\base\Application
and provides alternative client script implementations that work without jQuery.
- Main feature: Optional jQuery support controlled by
Yii::$app->useJquery
setting - Enhanced widgets: ActiveForm, ActiveField, and GridView components now support jQuery-free operation
- Comprehensive validation: All core validators can function without jQuery through new client script interfaces
Reviewed Changes
Copilot reviewed 83 out of 84 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
framework/widgets/ActiveForm.php | Adds client script interface and conditional jQuery dependency |
framework/widgets/ActiveField.php | Implements jQuery-optional validation and enhanced label rendering |
framework/web/View.php | Updates jQuery usage to reference global application setting |
framework/validators/*.php | Adds client script interfaces to all validators for jQuery independence |
framework/jquery/validators/*.php | New jQuery-specific client script implementations |
tests/framework/**/*.php | Comprehensive test coverage for new functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
This solution implements a way to make jQuery optional, while trying to keep BC as low as possible:
useJquery
property is added toyii\base\Application
, this allows you to control whether jQuery is used or not.I also think it would be a good idea to add the assets with CDN, and thus not depend on any package manager, if
YII_DEBUG
istrue
, it will be used without minification.Minimum BC:
CheckboxColum
=>protected function getHeaderCheckBoxName()
=>public function getHeaderCheckBoxName()
ActiveField
=>protected function isClientValidationEnabled()
=>public function isClientValidationEnabled()
ActiveField
=>protected function isAjaxValidationEnabled()
=>public function isAjaxValidationEnabled()
private function buildMimeTypeRegexp($mask)
=>public function buildMimeTypeRegexp($mask)
private function getIpParsePattern()
=>public function getIpParsePattern()
Two interfaces have been added to add the clientScript to validators, widgets, components, etc.
Fixed generation of labels, whether custom or not, when enclosedByLabel ===
false
, for bothcheckbox()
andradio()
.Example:
To install the assets, i added
php-forge/foxy
, which i maintain and allows you to install assets easily using thecomposer
installer. i think it would be great to use CDN to manage the assets, so we don't depend on anything external.Issues related: