Card Loading API #353
Card Loading API #353
Conversation
Codecov Report
@@ Coverage Diff @@
## master #353 +/- ##
=========================================
+ Coverage 52.1% 53.8% +1.69%
=========================================
Files 61 72 +11
Lines 3568 4130 +562
=========================================
+ Hits 1859 2222 +363
- Misses 1709 1908 +199
Continue to review full report at Codecov.
|
| @@ -149,13 +150,22 @@ | |||
| LOGIN_URL = '/user/login/' | |||
| LOGIN_REDIRECT_URL = '/' | |||
| ACCOUNT_EMAIL_REQUIRED = True | |||
| ACCOUNT_SIGNUP_FORM_CLASS= 'mangaki.forms.SignupForm' | |||
| ACCOUNT_SIGNUP_FORM_CLASS = 'mangaki.forms.SignupForm' | |||
There was a problem hiding this comment.
sorry, my OCD is incurable.
|
|
||
| class SlotCardTypes(enum.Enum): | ||
| popularity = 'popular' | ||
| controversy = 'controversial' |
There was a problem hiding this comment.
Why do we have different values than the name?
There was a problem hiding this comment.
At the start, I wanted to have something like: queryset[SlotCardTypes.popularity]()
But, due to DPP and random special treatment, that does not sound possible.
As a relic of the past, I kept the mapping. Shouldn't we uniformize them to match the queryset methods?
mangaki/mangaki/static/js/vote.js
Outdated
| @@ -210,6 +210,10 @@ Slot.prototype.fetch = function () { | |||
| /* Mosaic takes care of mapping Cards inside an element with Slots pointing to | |||
| * some remote URLs on the server. | |||
| */ | |||
| function buildSlotURL(category, slot_sort) { | |||
There was a problem hiding this comment.
Can you keep the documentation for the Mosaic function close to the Mosaic function?
mangaki/mangaki/views.py
Outdated
| @@ -317,8 +272,8 @@ def get_context_data(self, **kwargs): | |||
|
|
|||
| if sort_mode == 'mosaic' and not self.is_dpp: | |||
| context['object_list'] = [ | |||
| Work(title='Chargement…', ext_poster='/static/img/chiro.gif') | |||
| for _ in range(4) | |||
| (slot_sort_type, Work(title='Chargement…', ext_poster='/static/img/chiro.gif')) | |||
There was a problem hiding this comment.
Can we use {'sort_type': slot_sort_type, 'work': Work(title=...)} instead? Would make the template code more readable. BTW we should do {'work': Work(...)} for the other cases as well for consistency; that way we can easily get rid of works_mosaic.html.
| @@ -0,0 +1,13 @@ | |||
| <div class="row"> | |||
| <div class="col-xs-12 cards-grid"> | |||
| {% for slot_and_work in works %} | |||
There was a problem hiding this comment.
If we get rid of the tuple we can just do {% for work_meta in works %} in works.html then pass the work=work_meta.work slot_type=work_meta.slot_type in the {% include %} below. I believe Django uses None as the default for missing keys.
mangaki/mangaki/static/js/vote.js
Outdated
| this.slots = els.map(function (el, index) { | ||
| return new Slot('/data/card/' + category + '/' + (index + 1) + '.json'); | ||
| this.slots = els.map(function (el) { | ||
| return new Slot(buildSlotURL(category, $(el).data('slot-sort'))); |
There was a problem hiding this comment.
Please use attr('data-slot-sort')
There was a problem hiding this comment.
el.getAttribute('data-slot-sort')
With DRF's default documentation.
Fixes partially #130 (a plan is explained over there).
I think we should get rid of JSReverse in another PR which will clean out the JavaScript to use the public API.