-
-
Notifications
You must be signed in to change notification settings - Fork 22
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 filters #167
Add filters #167
Conversation
vjik
commented
Feb 19, 2024
•
edited
Loading
edited
Q | A |
---|---|
Is bugfix? | ❌ |
New feature? | ✔️ |
Breaks BC? | ✔️ |
Fix #159 |
PR Summary
|
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #167 +/- ##
============================================
- Coverage 70.79% 65.96% -4.84%
- Complexity 481 538 +57
============================================
Files 38 46 +8
Lines 1606 1807 +201
============================================
+ Hits 1137 1192 +55
- Misses 469 615 +146 ☔ View full report in Codecov by Sentry. |
src/UrlQueryReader.php
Outdated
{ | ||
public function get(string $name): ?string | ||
{ | ||
$value = $_GET[$name] ?? null; |
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.
Shouldn't we get it from PSR request instead of $_GET
?
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.
For use request we need somewhere find it.
-
Pass request to widget via special method. It's very not convenient.
-
Create
RequestProvider
and catch request sowewhere. Same implementation we use in Yii Input HTTP: https://github.com/yiisoft/input-http?tab=readme-ov-file#general-usage
But what is problem $_GET
usage?
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.
Now I remove UrlQueryReader
in favor UrlParameterProviderInterface
usage.
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.
But what is problem $_GET usage?
It's not available for many runtimes, especially event loop ones.