-
Notifications
You must be signed in to change notification settings - Fork 69
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
[DONE] More 3.5.x fixes #1043
[DONE] More 3.5.x fixes #1043
Conversation
Badatos
commented
Feb 1, 2024
•
edited
edited
- Remove everey useless "card-group" class as there is never several cards grouped with it
- Code formatting (remove useless spaces & indent)
- Add z-index to .card-footer-pod to prevent from hide by thumbnail below
- Add missing globals in some js files
- Video stats view highlights : color the th linked to the clicked stat from the video
- Now contrib.role is properly translated in video-info
- Remove the more_style block (redundant with page_extra_head)
- Replace some specific breadcrumb by generic {{page_title}} var
- Polish some templates (directs, directs_all, playlists
- Shrink vjs toolbar on very small screens (<=400px width)
- JS : test presence of favorite-btn-link
…cards grouped with it * Code formatting (remove useless spaces & indent) * Add z-index to .card-footer-pod to prevent from hide by thumbnail below * Add missing globals in some js files * Video stats view highlights : color the th linked to the clicked stat from the video * Now contrib.role is properly translated in video-info
* Replace some specific breadcrumb by generic {{page_title}} var * Polish some templates (directs, directs_all, playlists * Shrink vjs toolbar on very small screens (<=400pw width) * JS : test presence of favorite-btn-link
Merci @AymericJak & @SebastienCozeDev ! |
<span class="spinner-border text-primary" role="status"> | ||
</span> | ||
<i class="bi bi-play-btn" aria-hidden="true"></i> <span | ||
class="spinner-border text-primary" role="status"> </span> |
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.
Pourquoi un espace comme ça dans la span ?
Une utilité ? Ou on pourrait mettre
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.
C'est juste pour être certain de ne pas altérer le comportement précédent, comme ce n'est pas mon script, vu qu'il y avait des espace j'en ai laissé un ^^.
</h2> | ||
<button type="button" class="btn-close" data-bs-dismiss="modal" aria-label="${gettext( | ||
"Close", | ||
)}"></button> | ||
</div> | ||
<div class="modal-body"> | ||
<div class="text-center"> | ||
<span class="spinner-border text-primary" role="status"> | ||
</span> | ||
<span class="spinner-border text-primary" role="status"> </span> |
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.
Pareil
@@ -870,7 +870,8 @@ def get_filtered_videos_list(request, videos_list): | |||
return videos_list.distinct() | |||
|
|||
|
|||
def get_owners_has_instances(owners): | |||
def get_owners_has_instances(owners: list) -> list: |
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.
On peut même spécifier ce que contient la liste : -> list[Owner]
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.
Le souci c'est que la classe Owner n'est pas dans ce script, et ca m'embête de l'importer juste pour un doctype :/
pod/video/models.py
Outdated
def set_password(self): | ||
"""Encrypt the password if video is protected. An encrypted password cannot be re-encrypted.""" | ||
""" | ||
Encrypt the password if video is protected. | ||
|
||
An encrypted password cannot be re-encrypted. | ||
""" |
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.
Mettre None
serait une bonne idée ^^
@@ -870,7 +870,8 @@ def get_filtered_videos_list(request, videos_list): | |||
return videos_list.distinct() | |||
|
|||
|
|||
def get_owners_has_instances(owners): | |||
def get_owners_has_instances(owners: list) -> list: |
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.
Il serait peut-être préférable de mettre le type list[Owner]
* add @SInCE in som js scripts
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.
C'est bon pour moi !!
Merci beaucoup pour cette myriade de fix 😁
Par contre, pourquoi les tests ne passent plus ? |
File "/opt/hostedtoolcache/Python/3.9.18/x64/lib/python3.9/site-packages/django/template/loader_tags.py", line 212, in do_block
raise TemplateSyntaxError("'%s' tag with name '%s' appears more than once" % (bits[0], block_name))
django.template.exceptions.TemplateSyntaxError: 'block' tag with name 'page_content' appears more than once |
* now test translated strings * one of test users is now staff
C'est bon les tests repassent ;) J'en ai profité pour corriger le test sur RESTRICT_EDIT_IMPORT_VIDEO_ACCESS_TO_STAFF_ONLY : il était envoyé au template sans être utilisé. Maintenant la page affiche bien que l'acces est restreint. |
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.
Parfait pour moi