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
Draft: Suggestions in Zen mode #11187
base: main
Are you sure you want to change the base?
Conversation
@@ -25,7 +25,7 @@ | |||
{% else %} | |||
{% perm 'suggestion.delete' suggestion as user_can_delete_suggestion %} | |||
{% if user_can_vote_suggestion or user_can_accept_suggestion or user_can_delete_suggestion %} | |||
<div class="btn-float pull-right flip"> | |||
<div class="btn-float pull-right flip" style="top: -20px"> |
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.
Please avoid inline styles.
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.
yeah, I don't like it too, I wonder what should I do instead? Changing the original class doesn't sound like a good idea either, I guess I could create new one?
# Handle accepting/deleting suggestions | ||
_obj, unit_set, _context = parse_path_units( | ||
request, path, (Translation, ProjectLanguage, CategoryLanguage) | ||
) |
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.
This is already done at top of this function.
or "spam" in request.POST | ||
or "upvote" in request.POST | ||
or "downvote" in request.POST | ||
) |
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.
This is essentially a copy of code from translate
function, it should be shared.
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.
Can I just put such a function in the same file, or it should go to some utils.py file?
|
||
unit = checksum_form.cleaned_data["unit"] | ||
# We are ignoring any redirects and responses here | ||
_response = handle_suggestions(request, unit, "", "") |
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.
If the return value is not needed, there is no need to store it in a variable.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11187 +/- ##
==========================================
- Coverage 90.82% 90.74% -0.08%
==========================================
Files 554 559 +5
Lines 57306 57680 +374
Branches 9122 9212 +90
==========================================
+ Hits 52046 52341 +295
- Misses 3640 3704 +64
- Partials 1620 1635 +15
|
Thanks for the review! I want to continue working on it, just got interrupted somewhat. |
This pull request has been automatically marked as stale because there wasn’t any recent activity. It will be closed soon if no further action occurs. Thank you for your contributions! |
meow, bot |
Proposed changes
Solves #671, kind of.
At the time of writing, it is functional, but probably need to move most events to javascript to avoid resending Zen each time.
Also, I went via always show suggestions route, but I think that probably may be configured.
Checklist
Other information