-
Notifications
You must be signed in to change notification settings - Fork 107
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
[Feat] add category filter on review page #1145
Conversation
Codecov ReportAttention:
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1145 +/- ##
==========================================
+ Coverage 69.74% 70.59% +0.84%
==========================================
Files 85 85
Lines 3378 3394 +16
==========================================
+ Hits 2356 2396 +40
+ Misses 1022 998 -24
☔ View full report in Codecov by Sentry. |
outcome Dashboard._.PyCon.Taiwan.-.Google.Chrome.2023-07-04.21-57-26.mp4 |
6abfbc6
to
bad1dec
Compare
@josix updated outcome
|
src/reviews/views.py
Outdated
}) | ||
print(context) |
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 guess this is used for debugging? could you help remove it, thanks!
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.
Resolved in daa9f1b
<option value="?order=count">ALL</option> | ||
{% else %} | ||
<option value="?order=count&category={{ category }}">{{ category }}</option> | ||
<option value="?order=count">ALL</option> | ||
{% endif %} | ||
{% for category_option in category_options %} | ||
{% if category != category_option %} | ||
<option value="?order=count&category={{ category_option }}">{{ category_option }}</option> |
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.
issue: the value order=count
here will cause the proposals ordered only by the count after selecting the category even the user has chosen to ordered by other field like lang
, count
, etc. I guess we need to pass the query parameter from URL.
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.
Resolved in daa9f1b
use a trick to retain both order={}&category={} query params.
… work at the same time
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.
LGTM, Sorry that I didn't notice that there is an update for the last review.
Types of changes
Thanks for sending a pull request! Please fill in the following content to let us know better about this change.
Please put an
x
in the box that appliesDescription
Describe what the change is
Add category filed to proposal table. When you click on a category, the list of proposals will be filtered by the category you selected.
Steps to Test This Pull Request
Steps to reproduce the behavior:
Expected behavior
Show the proposal list which was be filtered.
Related Issue
If applicable, refernce to the issue related to this pull request.
More Information
Screenshots
If applicable, add screenshots to help explain your problem.
Additional context
Add any other context about the problem here. You may also want to refer
to how to wirte the perfect pull request
2022-04-09.4.10.36.mov