feat(ui): enhance media list views display#36
Conversation
PascalRepond
commented
Jan 3, 2026
- Improved layout, colours and buttons for better user experience.
- Created templatetags for media icons to standardize icon rendering across the app.
- Updated relevant views and templates to integrate new features seamlessly.
📝 WalkthroughWalkthroughAdds a media template tags module, multiple new UI partials, refactors media list/grid templates and review truncation/styling, updates tests and a form placeholder, and removes HTMX inline-edit views, routes, and editable partials for media score, status, and review date. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tests/core/test_views.py (1)
346-358: Stale comment references old threshold.Line 356 still mentions "less than 20 words" but the threshold was updated to 30 words in
media-review-clamped.html. The comment should be updated for consistency.🔎 Proposed fix
content = response.content.decode("utf-8") - # With less than 20 words, the 'See more' button should not appear + # With less than 30 words, the 'See more' button should not appear assert "See more" not in content
🧹 Nitpick comments (3)
src/templates/partials/media-cover.html (1)
5-9: Consider removing redundant width/height attributes.The explicit
width="96"andheight="128"attributes duplicate the sizing already defined by Tailwind classesw-24 h-32. While these attributes can help with layout shift prevention, they create a maintenance burden if the design changes.If you need explicit dimensions for performance (CLS prevention), consider extracting these values to a centralized configuration.
💡 Optional refactor to remove duplication
- <img src="{{ media.cover.url }}" - alt="{% translate "Cover of" %} {{ media.title }}" - width="96" - height="128" - class="w-full h-full object-contain rounded"> + <img src="{{ media.cover.url }}" + alt="{% translate "Cover of" %} {{ media.title }}" + class="w-full h-full object-contain rounded">src/templates/partials/media-items.html (2)
23-27: Consider using the media-score-badge partial for consistency.The grid view embeds the score badge markup inline, while the list view uses
{% include "partials/media-score-badge.html" %}(line 69). This creates duplication and potential maintenance issues if the badge styling needs to change.🔎 Refactor to use the shared partial
- {% if media.score %} - <span class="absolute top-2 right-2 badge gap-0 h-auto w-auto"> - {{ media.score }}{% heroicon_mini "star" class="h-4" %} - {{ media.get_score_display }} - </span> - {% endif %} + <div class="absolute top-2 right-2"> + {% include "partials/media-score-badge.html" with extra_class="h-auto w-auto" %} + </div>Note: You may need to adjust the
extra_classparameter in the partial to support positioning classes.
71-73: Replace inline style with Tailwind utility class.The
max-width: 50reminline style should use Tailwind'smax-w-[50rem]utility class for consistency with the rest of the codebase.🔎 Proposed refactor
- <div id="review-cell-{{ media.id }}" - class="text-sm review-text" - style="max-width: 50rem">{% include "partials/media-review-clamped.html" %}</div> + <div id="review-cell-{{ media.id }}" + class="text-sm review-text max-w-[50rem]">{% include "partials/media-review-clamped.html" %}</div>
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (5)
- src/core/urls.py
- src/templates/partials/score-editable.html
- src/templates/partials/review-date-editable.html
- src/core/views.py
- src/templates/partials/status-editable.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-edit-button.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-cover.html
🧬 Code graph analysis (1)
src/templates/partials/media-edit-button.html (1)
src/core/views.py (1)
media_edit(135-170)
🔇 Additional comments (12)
src/templates/partials/media-review-clamped.html (1)
3-8: LGTM!The truncation threshold update to 30 words is consistent between the
truncatewords_htmlfilter and thewordcountconditional check. The test file has been appropriately updated to use 35 words to validate the threshold behavior.src/tests/core/test_views.py (1)
359-371: LGTM!The test correctly uses 35 words to exceed the new 30-word threshold, and the comment accurately reflects the updated behavior.
src/core/templatetags/media_tags.py (2)
8-52: LGTM!The
media_iconinclusion tag is well-structured with clear documentation, comprehensive mappings, and sensible defaults for unknown media types and sizes.
55-76: LGTM!The
status_badge_classfilter provides a clean mapping from status values to DaisyUI badge classes with a sensiblebadge-ghostfallback.src/templates/partials/media-review-full.html (1)
1-7: LGTM!The margin removal aligns button styling with the clamped review partial for visual consistency.
src/templates/partials/media-icon.html (1)
1-8: LGTM!Clean conditional rendering with proper variant handling and fallback to the outline variant. The template correctly loads the heroicons library.
src/templates/partials/media-status-badge.html (1)
1-6: LGTM!The partial cleanly integrates the
status_badge_classfilter with optional size support and leverages Django'sget_status_displayfor human-readable status text.src/templates/partials/media-score-badge.html (1)
3-13: LGTM with minor observation.The score badge renders correctly with conditional sizing for the star icon. The
gap-0class creates a compact layout which appears intentional for the badge styling.One minor suggestion: The conditional icon sizing could be simplified using a variable, but the current implementation is clear and maintainable.
src/templates/partials/media-list.html (2)
6-6: Grid layout enhancement looks good.Increasing the grid columns from 4 to 5 at the
xlbreakpoint provides better space utilization on larger screens.
14-19: Table header responsive design is well-structured.The progressive disclosure pattern (hiding columns on smaller screens) effectively manages information density across breakpoints. The visibility rules correctly match the corresponding cells in
media-items.html.src/templates/partials/media-cover.html (1)
15-17: No action needed. Themedia_icontemplate tag is properly registered insrc/core/templatetags/media_tags.pywith@register.inclusion_tag()and correctly accepts themedia_typeandsizeparameters. The template usage{% media_icon media.media_type size="sm" %}is valid.Likely an incorrect or invalid review comment.
src/templates/partials/media-contributors.html (1)
5-12: All hx-include target elements are present and correctly scoped.The seven input elements referenced in the
hx-includeattribute (#view-mode-input,#sort,#type,#status,#score,#review-from,#review-to) are all defined in the parentmedia.htmltemplate and its included partials. They are rendered before the media items partial and will be available in the DOM when HTMX requests are triggered. The samehx-includepattern is consistently used across other HTMX-enabled elements in the interface.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/core/templatetags/media_tags.py (1)
9-52: Consider validating the variant parameter.The
variantparameter is passed through without validation. If the template expects specific values (e.g., "outline", "solid", "mini"), passing an invalid variant could cause rendering issues or silent failures.🔎 Proposed validation
@register.inclusion_tag("partials/media-icon.html") def media_icon(media_type, size="sm", variant="outline"): """ Render a heroicon based on media type. Args: media_type: The type of media (BOOK, GAME, MUSIC, etc.) size: Icon size (sm, md, lg) - default 'sm' variant: Icon variant (outline, solid, mini) - default 'outline' Returns: Context dict with icon_name, size_class, and variant Example usage: {% load media_tags %} {% media_icon media.media_type %} {% media_icon media.media_type size="md" variant="solid" %} """ + # Validate variant parameter + valid_variants = {"outline", "solid", "mini"} + if variant not in valid_variants: + variant = "outline" + # Mapping of media types to heroicon names media_type_icons = {src/templates/partials/media-score-badge.html (1)
4-4: Reconsider thegap-0class for better visual spacing.The badge contains a numeric score, a star icon, and descriptive text. Using
gap-0removes spacing between these elements, which may result in cramped appearance. Consider usinggap-1orgap-0.5for better readability.🔎 Proposed adjustment
- <span class="badge {% if size == 'sm' %}badge-sm{% endif %} gap-0 {{ extra_class }}"> + <span class="badge {% if size == 'sm' %}badge-sm{% endif %} gap-1 {{ extra_class }}">
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
src/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (5)
- src/templates/partials/review-date-editable.html
- src/core/urls.py
- src/core/views.py
- src/templates/partials/status-editable.html
- src/templates/partials/score-editable.html
🚧 Files skipped from review as they are similar to previous changes (5)
- src/templates/partials/media-icon.html
- src/templates/partials/media-edit-button.html
- src/templates/partials/media-list.html
- src/templates/partials/media-contributors.html
- src/templates/partials/media-status-badge.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-score-badge.html
🧬 Code graph analysis (1)
src/tests/core/test_views.py (1)
src/tests/conftest.py (1)
logged_in_client(65-68)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (12)
src/templates/partials/media-review-full.html (1)
4-7: LGTM!The removal of the
mt-1class is a minor styling refinement that improves visual consistency. The HTMX functionality remains intact.src/templates/partials/media-review-clamped.html (1)
3-8: LGTM!The increased truncation threshold from 20 to 30 words provides users with more context before requiring expansion. The changes are consistent:
- Truncation limit and conditional threshold both updated to 30 words
- Test coverage properly updated to reflect the new threshold
- Button styling aligned with other UI refinements
src/tests/core/test_views.py (1)
356-371: LGTM!The test updates correctly reflect the increased 30-word threshold for review truncation:
- Comments updated to reference the new threshold
- Test data adjusted to use 35 words (exceeding the 30-word limit)
- Changes are consistent with the template updates in
media-review-clamped.htmlNote: The static analysis hint about the unused
dbparameter on line 359 is a false positive—this is a standard pytest fixture pattern for ensuring database access in tests.src/templates/partials/media-cover.html (1)
1-16: Themedia_icontemplate tag is properly defined insrc/core/templatetags/media_tags.pyas an inclusion tag that renders heroicons based on media type. The implementation includes:
- All required media types mapped (BOOK, GAME, MUSIC, COMIC, FILM, TV, PERF, BROADCAST) with a fallback to "question-mark-circle"
- Correct parameter handling (media_type, size, variant)
- Proper Django template library registration
- Template usage in media-cover.html is valid
No issues found.
src/core/forms.py (1)
62-62: Correct the placeholder format from "MM-YYYY" to "YYYY-MM".The django-partial-date library supports YYYY, YYYY-MM, and YYYY-MM-DD formats. The placeholder text incorrectly lists "MM-YYYY" instead of the correct "YYYY-MM" format. Update the placeholder to: "YYYY, YYYY-MM, or YYYY-MM-DD".
Likely an incorrect or invalid review comment.
src/templates/partials/media-items.html (5)
2-2: Good addition.Loading the
media_tagsmodule enables the newmedia_icontag andstatus_badge_classfilter used throughout this template.
12-12: Verify the switch fromobject-covertoobject-contain.Changing from
object-covertoobject-containalters how images are displayed.object-containwill preserve the entire image within the container (potentially with letterboxing), whileobject-coverfills the container (potentially cropping). Ensure this change aligns with your design intent.
19-19: Proper usage of the new media_icon tag.The
media_icontag is correctly invoked with the media type and size parameter.
64-64: Well-structured responsive layout.The progressive disclosure pattern using
hiddenwith breakpoint-specifictable-cellclasses provides a good mobile-first responsive experience, showing more columns as screen size increases.Also applies to: 73-73, 75-75
22-22: All referenced partial templates exist in the repository and are properly implemented:
partials/media-score-badge.html✓partials/media-contributors.html✓partials/media-edit-button.html✓partials/media-status-badge.html✓partials/media-review-clamped.html✓partials/media-cover.html✓No action needed—all includes will render correctly.
Likely an incorrect or invalid review comment.
src/core/templatetags/media_tags.py (2)
55-76: Implementation looks good.The status badge class mapping correctly covers all Media model status choices (PLANNED, IN_PROGRESS, COMPLETED, PAUSED, DNF) and provides a sensible fallback with "badge-ghost".
8-8: The template filepartials/media-icon.htmlexists atsrc/templates/partials/media-icon.htmland is correctly referenced by the inclusion tag.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/templates/partials/media-items.html (1)
67-67: Fix invalid Tailwind CSS class (duplicate issue).The class
max-w-200is not a valid Tailwind CSS class. This was previously flagged but remains unfixed. Use a valid Tailwind class likemax-w-prose,max-w-md, or an arbitrary value likemax-w-[200px].🔎 Suggested fixes
Option 1: Use a standard Tailwind class (recommended):
- <div id="review-cell-{{ media.id }}" class="text-sm max-w-200 review-text"> + <div id="review-cell-{{ media.id }}" class="text-sm max-w-prose review-text">Option 2: Use arbitrary value (Tailwind 3.x):
- <div id="review-cell-{{ media.id }}" class="text-sm max-w-200 review-text"> + <div id="review-cell-{{ media.id }}" class="text-sm max-w-[200px] review-text">
🧹 Nitpick comments (2)
src/core/templatetags/media_tags.py (1)
8-23: Document thevariantparameter in the docstring.The function accepts a
variantparameter (line 9) that controls icon style (solid, mini, or outline), but it's not documented in the docstring Args section.🔎 Proposed documentation update
@register.inclusion_tag("partials/media-icon.html") def media_icon(media_type, size="sm", variant="outline"): """ Render a heroicon based on media type. Args: media_type: The type of media (BOOK, GAME, MUSIC, etc.) size: Icon size (sm, md, lg) - default 'sm' + variant: Icon variant (solid, mini, outline) - default 'outline' Returns: Context dict with icon_name and size_class Example usage: {% load media_tags %} {% media_icon media.media_type size="md" %} """src/templates/partials/media-items.html (1)
35-41: Remove redundanttext-xsclass.The parent
<div>on line 35 hasclass="text-xs", but the child<div>on line 37 also hastext-xsin its classes. The parent class is redundant since the text size is already defined on the child element.🔎 Proposed fix
- <div class="text-xs"> + <div> {% if media.review %} <div id="review-cell-{{ media.id }}" class="text-xs review-text"> {% include "partials/media-review-clamped.html" %}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.vscode/settings.jsonsrc/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (5)
- src/core/views.py
- src/core/urls.py
- src/templates/partials/score-editable.html
- src/templates/partials/review-date-editable.html
- src/templates/partials/status-editable.html
✅ Files skipped from review due to trivial changes (1)
- .vscode/settings.json
🚧 Files skipped from review as they are similar to previous changes (7)
- src/templates/partials/media-cover.html
- src/templates/partials/media-edit-button.html
- src/templates/partials/media-list.html
- src/templates/partials/media-contributors.html
- src/templates/partials/media-status-badge.html
- src/templates/partials/media-review-clamped.html
- src/templates/partials/media-score-badge.html
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-review-full.html
🧬 Code graph analysis (2)
src/templates/partials/media-items.html (3)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
Migration(9-41)src/core/migrations/0008_media_review_rendered_alter_media_review.py (1)
Migration(24-43)src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)
src/tests/core/test_views.py (1)
src/tests/conftest.py (1)
logged_in_client(65-68)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (8)
src/core/forms.py (1)
62-62: LGTM: ISO 8601 date format adoption.The placeholder change from "DD-MM-YYYY" to "YYYY-MM-DD" aligns with the ISO 8601 standard, improving consistency across the application.
src/templates/partials/media-review-full.html (1)
4-4: Verify negative margin doesn't cause layout issues.The button now uses a negative top margin (
-mt-3), which pulls it upward by 0.75rem. Ensure this doesn't cause overlapping or clipping issues with content above it, especially across different screen sizes and review lengths.src/tests/core/test_views.py (1)
356-357: LGTM: Test thresholds updated consistently.The word count thresholds for the "See more" button have been updated from 20 to 30 words (minimum) and 50 words (expansion threshold), with test data adjusted accordingly. The logic is consistent and correct.
Note: The static analysis warning about the unused
dbparameter on line 359 is a false positive—this is the standard pytest fixture pattern for ensuring database availability.Also applies to: 361-361, 370-371
src/core/templatetags/media_tags.py (2)
25-50: LGTM: Well-structured icon mapping with appropriate fallbacks.The icon mapping covers all expected media types and includes a sensible fallback ("question-mark-circle") for unknown types. The size classes are properly mapped to Tailwind utilities.
53-74: LGTM: Status badge mapping is complete and well-documented.The filter correctly maps all status values to DaisyUI badge classes with an appropriate fallback.
src/templates/partials/media-icon.html (1)
1-8: LGTM: Clean and straightforward icon rendering.The template correctly handles the three icon variants (solid, mini, outline) with a sensible default fallback to outline. The implementation integrates well with the
media_icontemplate tag.src/templates/partials/media-items.html (2)
22-22: All included partial templates exist and are correctly located insrc/templates/partials/.
2-2: Themedia_tagstemplate tag library exists atsrc/core/templatetags/media_tags.pyand themedia_icontag is properly registered with the@register.inclusion_tagdecorator. The{% load media_tags %}statement is valid and will correctly provide themedia_icontag used in the template.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/templates/partials/media-score-badge.html (1)
1-9: Remove unused 'size' parameter from documentation.The template documents a
sizeparameter on line 2, but it's never used in the implementation. Either implement size-based styling or remove it from the documentation to avoid confusion.🔎 Suggested fix
{# Renders a score badge if score exists #} -{# Parameters: media, size (optional: 'sm' or default), extra_class (optional) #} +{# Parameters: media, extra_class (optional) #} {% if media.score %} <span class="badge badge-neutral gap-0 {{ extra_class }}"> {{ media.score }} {% heroicon_mini "star" class="fill-orange-400 h-4" %} - {{ media.get_score_display }} </span> {% endif %}src/templates/partials/media-items.html (1)
21-23: Remove ineffective margin class on absolutely positioned element.The
extra_class="mb-2"passed to the score badge partial has no effect because the parent<div>usesabsolutepositioning. Margins don't apply to absolutely positioned elements in the same way.🔎 Suggested fix
{% if media.score %} - <div class="absolute top-2 right-2">{% include "partials/media-score-badge.html" with extra_class="mb-2" %}</div> + <div class="absolute top-2 right-2">{% include "partials/media-score-badge.html" %}</div> {% endif %}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.vscode/settings.jsonsrc/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (5)
- src/templates/partials/status-editable.html
- src/templates/partials/score-editable.html
- src/core/views.py
- src/templates/partials/review-date-editable.html
- src/core/urls.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/templates/partials/media-edit-button.html
- src/core/templatetags/media_tags.py
- .vscode/settings.json
- src/templates/partials/media-contributors.html
- src/templates/partials/media-review-clamped.html
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-status-badge.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-icon.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.
Applied to files:
src/templates/partials/media-status-badge.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-icon.html
🧬 Code graph analysis (1)
src/templates/partials/media-items.html (4)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
Migration(9-41)src/core/migrations/0008_media_review_rendered_alter_media_review.py (1)
Migration(24-43)src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)src/core/migrations/0007_alter_media_score.py (1)
Migration(6-18)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (10)
src/core/forms.py (1)
62-62: LGTM! ISO 8601 date format is clearer.The updated placeholder text now includes YYYY-MM-DD (ISO 8601 standard) instead of DD-MM-YYYY, which is more universally recognized and reduces ambiguity for international users.
src/templates/partials/media-icon.html (1)
1-2: LGTM! Clean icon rendering implementation.The template correctly loads the heroicons library and renders an outline icon using context variables provided by the media_icon template tag. This standardizes icon rendering across the application as intended.
src/tests/core/test_views.py (1)
356-371: LGTM! Test thresholds correctly updated.The test expectations have been properly updated to reflect the new UI threshold of 50 words for review truncation (previously 20 words). The long_review fixture now generates 55 words to ensure the "See more" button appears, while the short review test correctly expects no button for reviews under 30 words.
Note: The Ruff warning about the unused
dbargument on line 359 is a false positive—pytest fixtures are required even when not explicitly referenced in the test body.src/templates/partials/media-status-badge.html (1)
1-4: LGTM! Well-documented status badge implementation.The template correctly uses the status_badge_class filter to apply appropriate styling and Django's get_status_display method for human-readable status text. The inline documentation clearly explains the partial's purpose and required parameters.
src/templates/partials/media-review-full.html (1)
4-4: LGTM! Button styling updated for enhanced UI.The button styling has been updated from
btn-ghosttobtn-outlinefor better visual contrast, and the margin adjusted frommt-1to-mt-3for tighter spacing with the review content. This aligns with the PR's objective to improve layout and visual presentation.src/templates/partials/media-cover.html (1)
1-16: LGTM! Clean and accessible media cover implementation.The partial correctly handles both cover images and placeholder states, with proper i18n support and icon rendering via the new template tag.
src/templates/partials/media-list.html (2)
6-6: LGTM! Increased grid density for larger screens.Expanding to 5 columns on xl screens makes good use of available space while maintaining appropriate card sizes.
14-19: LGTM! Well-structured responsive table headers.The responsive visibility classes and fixed widths create a clean, mobile-friendly table layout that progressively reveals more information on larger screens.
src/templates/partials/media-items.html (2)
2-2: LGTM! Correct template tag loading.The
media_tagstemplate library is properly loaded for the new icon rendering functionality.
47-82: LGTM! Well-structured table row layout.The table row implementation correctly uses the new partials and maintains good responsive design with appropriate column visibility classes. The em-dash for missing review dates is a nice UX touch.
Note: The
size="sm"parameter passed tomedia-score-badge.htmlon line 63 is currently unused (as noted in the review of that partial).
3491361 to
ac8480b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/tests/core/test_views.py (1)
346-357: Clarify comment inconsistency.The comment on line 356 mentions "less than 30 words," but the actual threshold in
media-review-clamped.htmlis 50 words. The test review "Short review" is only 2 words, so the test will pass regardless, but the comment should match the actual template threshold for clarity.🔎 Proposed fix
- # With less than 30 words, the 'See more' button should not appear + # With less than 50 words, the 'See more' button should not appear assert "See more" not in content
🧹 Nitpick comments (2)
src/tests/core/test_views.py (1)
359-371: Remove unuseddbfixture parameter.The static analysis tool correctly identifies that the
dbparameter is unused in this test. Since the test creates aMediaobject, thedbfixture is implicitly available through pytest-django's automatic fixture discovery.🔎 Proposed fix
- def test_media_review_clamped_with_long_review(self, logged_in_client, db): + def test_media_review_clamped_with_long_review(self, logged_in_client): """Clamped review with long text shows 'See more' button."""src/templates/partials/media-contributors.html (1)
5-12: Use real URLs inhreffor progressive enhancement.The
href="#"on line 5 means these links do nothing when JavaScript/HTMX fails to load. For better accessibility and progressive enhancement, use the actual URL ({% url 'home' %}?contributor={{ contributor.id }}) in thehrefattribute and let HTMX intercept the click.🔎 Proposed fix
<a href="#" - class="link link-hover contributor-link" + <a href="{% url 'home' %}?contributor={{ contributor.id }}" + class="link link-hover contributor-link" data-contributor-id="{{ contributor.id }}"
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.vscode/settings.jsonsrc/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (5)
- src/templates/partials/score-editable.html
- src/templates/partials/status-editable.html
- src/core/views.py
- src/templates/partials/review-date-editable.html
- src/core/urls.py
🚧 Files skipped from review as they are similar to previous changes (5)
- src/templates/partials/media-status-badge.html
- src/core/templatetags/media_tags.py
- src/templates/partials/media-list.html
- src/templates/partials/media-cover.html
- src/templates/partials/media-icon.html
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-edit-button.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-review-full.html
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.
Applied to files:
src/templates/partials/media-edit-button.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-review-full.html
🧬 Code graph analysis (2)
src/templates/partials/media-edit-button.html (1)
src/core/views.py (1)
media_edit(135-170)
src/templates/partials/media-items.html (4)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
Migration(9-41)src/core/migrations/0008_media_review_rendered_alter_media_review.py (1)
Migration(24-43)src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)src/core/migrations/0007_alter_media_score.py (1)
Migration(6-18)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (7)
src/templates/partials/media-review-full.html (1)
4-7: Verify the negative margin doesn't cause layout issues.The button styling has been updated for consistency with the clamped review variant. The change from
mt-1(4px) to-mt-3(-12px) introduces a negative top margin that pulls the button upward.Ensure this negative margin works correctly across all contexts where this partial is rendered, particularly when the review text is short or when other UI elements are nearby.
.vscode/settings.json (1)
14-14: LGTM!Adding
H006to the ignore list is reasonable for responsive web designs where image dimensions are handled via CSS rather than inline HTML attributes.src/tests/core/test_views.py (1)
361-371: LGTM!The test correctly validates the updated truncation threshold. Using 55 words for the long review and checking for the "See more" button with a threshold of 50 words ensures the functionality works as expected.
src/templates/partials/media-review-clamped.html (1)
3-8: LGTM!The increased truncation threshold from 20 to 50 words provides users with more preview text before requiring expansion, which improves the reading experience. The changes are consistent with the updated tests and the styling matches the full review partial.
As noted in the review of
media-review-full.html, ensure the negative margin (-mt-3) on the button works well in all layout contexts.src/core/forms.py (1)
62-62: Backend date parsing confirmed to support YYYY-MM-DD format.The placeholder text correctly specifies the three formats (
YYYY,MM-YYYY, andYYYY-MM-DD) that the backendPartialDateFieldfromdjango-partial-date(>=1.3.2) accepts. The change fromDD-MM-YYYYto ISO 8601 format is properly supported and improves international consistency.src/templates/partials/media-edit-button.html (1)
1-8: LGTM!The edit button partial is well-implemented with proper template tag loading, good accessibility (aria-label), and internationalization support. The previous issue regarding the missing heroicons load has been addressed.
src/templates/partials/media-items.html (1)
4-84: Well-structured UI enhancement!The restructured grid and list views are well-implemented with:
- Clear separation between grid and list/table layouts
- Proper use of new partials for badges, covers, and contributors
- Good responsive design with conditional column visibility
- Consistent styling and proper accessibility
The numeric Tailwind utilities (e.g.,
max-w-200on line 65) are correctly used per Tailwind v4 conventions.Based on learnings, Tailwind v4.1.11 supports arbitrary numeric utilities.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/templates/partials/media-items.html (1)
1-2: Missing{% load heroicons %}for the heroicon tag on line 15.The heroicons library is still not loaded, but line 15 uses
{% heroicon_outline "photo" %}. This will cause aTemplateSyntaxErrorwhen rendering media items without covers.🔎 Proposed fix
{% load i18n %} {% load media_tags %} +{% load heroicons %}
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
.vscode/settings.jsonsrc/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (5)
- src/templates/partials/score-editable.html
- src/templates/partials/status-editable.html
- src/templates/partials/review-date-editable.html
- src/core/urls.py
- src/core/views.py
🚧 Files skipped from review as they are similar to previous changes (11)
- src/templates/partials/media-icon.html
- src/templates/partials/media-review-full.html
- src/templates/partials/media-review-clamped.html
- src/templates/partials/media-status-badge.html
- src/templates/partials/media-edit-button.html
- src/templates/partials/media-cover.html
- .vscode/settings.json
- src/templates/partials/media-score-badge.html
- src/templates/partials/media-list.html
- src/templates/partials/media-contributors.html
- src/core/templatetags/media_tags.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.
Applied to files:
src/templates/partials/media-items.html
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-items.html
🧬 Code graph analysis (1)
src/tests/core/test_views.py (1)
src/tests/conftest.py (1)
logged_in_client(65-68)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (3)
src/core/forms.py (1)
62-62: LGTM! Improved date format placeholder.The change to ISO 8601 format (YYYY-MM-DD) is more standard and unambiguous compared to the previous DD-MM-YYYY format.
src/tests/core/test_views.py (1)
359-371: Template threshold matches test expectations.The template correctly uses a 50-word threshold:
truncatewords_html:50truncates content andwordcount > 50gates the "See more" button display. Both test cases validate this threshold correctly (30-word review stays unclamped, 55-word review shows the button).Note: The static analysis warning about the unused
dbargument on line 359 is a false positive. This is a standard pytest-django fixture pattern that ensures database access is available for the test.src/templates/partials/media-items.html (1)
48-48: Verify accessibility in the media-cover.html partial.The included
media-cover.htmlpartial likely renders a media type badge similar to the grid view. Ensure it includes accessible text labels for icons (e.g.,aria-labelor visible text) so screen reader users can identify the media type.
d3dc55d to
afd2e74
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/templates/partials/media-items.html (1)
1-3: Missing{% load heroicons %}for the heroicon tag.The template uses
{% heroicon_outline "photo" %}on line 15, but heroicons is not explicitly loaded. Based on retrieved learnings, heroicons is configured in TEMPLATES builtins and should be globally available. However, if you encounter template errors, add the load directive.Please verify that heroicons is indeed configured in your Django TEMPLATES settings builtins. If so, no change is needed. If not, add the load:
{% load i18n %} {% load media_tags %} +{% load heroicons %}
🧹 Nitpick comments (2)
src/core/templatetags/media_tags.py (1)
25-44: Consider extracting mappings as module-level constants.The dictionaries
media_type_iconsandsize_classesare recreated on every tag invocation. For a minor performance improvement and easier maintenance, you could extract them as module-level constants.🔎 Proposed refactor
from django import template register = template.Library() +MEDIA_TYPE_ICONS = { + "BOOK": "book-open", + "GAME": "computer-desktop", + "MUSIC": "musical-note", + "COMIC": "book-open", + "FILM": "film", + "TV": "tv", + "PERF": "ticket", + "BROADCAST": "microphone", +} + +SIZE_CLASSES = { + "sm": "w-4 h-4", + "md": "w-6 h-6", + "lg": "w-8 h-8", +} + @register.inclusion_tag("partials/media-icon.html") def media_icon(media_type, size="sm"): # ... docstring ... - media_type_icons = { - "BOOK": "book-open", - # ... - } - size_classes = { - "sm": "w-4 h-4", - # ... - } - icon_name = media_type_icons.get(media_type, "question-mark-circle") - size_class = size_classes.get(size, "w-4 h-4") + icon_name = MEDIA_TYPE_ICONS.get(media_type, "question-mark-circle") + size_class = SIZE_CLASSES.get(size, SIZE_CLASSES["sm"]) # ...src/tests/core/test_views.py (1)
359-371: Verify the word count threshold logic is correctly tested.The test generates 55 words and expects the "See more" button to appear when the review exceeds 50 words. This logic is sound. However, consider adding a boundary test case for exactly 50 words to ensure the threshold behavior is precisely defined (should 50 words show the button or not?).
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.vscode/settings.jsonsrc/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/backup_manage.htmlsrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (6)
- src/templates/partials/score-editable.html
- src/templates/backup_manage.html
- src/core/urls.py
- src/templates/partials/review-date-editable.html
- src/core/views.py
- src/templates/partials/status-editable.html
🚧 Files skipped from review as they are similar to previous changes (10)
- src/templates/partials/media-edit-button.html
- src/templates/partials/media-review-clamped.html
- src/templates/partials/media-contributors.html
- src/templates/partials/media-status-badge.html
- src/core/forms.py
- src/templates/partials/media-score-badge.html
- src/templates/partials/media-icon.html
- src/templates/partials/media-list.html
- .vscode/settings.json
- src/templates/partials/media-review-full.html
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-cover.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In Django projects, configuring heroicons in TEMPLATES builtins makes heroicons.templatetags.heroicons available in all templates without needing {% load heroicons %}. For template reviews in this codebase, assume heroicon tags work globally in HTML templates under templates/, so avoid requiring explicit loads. If a template still uses {% load heroicons %}, assess whether the load is unnecessary and can be removed; ensure no conflicts arise from global tags.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-cover.html
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-cover.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In the PascalRepond/datakult repository, heroicons is configured in Django's TEMPLATES settings builtins (likely as "heroicons.templatetags.heroicons"), making all heroicon template tags globally available without requiring `{% load heroicons %}` in individual templates.
Applied to files:
src/core/templatetags/media_tags.py
🧬 Code graph analysis (1)
src/templates/partials/media-items.html (3)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
Migration(9-41)src/core/migrations/0008_media_review_rendered_alter_media_review.py (1)
Migration(24-43)src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (4)
src/templates/partials/media-cover.html (1)
1-16: LGTM! Clean and accessible partial.The template properly handles both cover image and placeholder states, includes translated alt text for accessibility, and adds an
aria-labelto the media type badge. Based on learnings, heroicons is globally available via TEMPLATES builtins, so no explicit load is needed.src/core/templatetags/media_tags.py (1)
52-73: LGTM!The filter cleanly maps status values to DaisyUI badge classes with a sensible
badge-ghostfallback for unknown statuses. Well documented.src/templates/partials/media-items.html (2)
7-42: LGTM! Well-structured card layout.The grid view uses a clean card structure with proper separation of concerns via partials. The accessibility concern from previous reviews has been addressed with
aria-labelon the media type badge.
46-84: LGTM! Clean responsive table layout.The list view properly uses responsive utility classes (
hidden md:table-cell, etc.) to hide less critical columns on smaller screens. Themax-w-200class is valid in Tailwind CSS v4. Based on learnings, this maps tocalc(var(--spacing) * 200).
- Improved layout, colours and buttons for better user experience. - Created templatetags for media icons to standardize icon rendering across the app. - Updated relevant views and templates to integrate new features seamlessly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/tests/core/test_views.py (1)
359-359: Optional: Consider removing unuseddbfixture.The static analysis tool flags the
dbparameter as unused. WhileMedia.objects.create()implicitly uses the database, thedbfixture may not be strictly necessary here. However, this is a common pattern in pytest-django tests to ensure database access, so removing it is optional.🔎 Optional fix to remove unused parameter
- def test_media_review_clamped_with_long_review(self, logged_in_client, db): + def test_media_review_clamped_with_long_review(self, logged_in_client):Note: Only apply this change if you're certain the test doesn't require explicit database fixture setup. The current pattern is acceptable in pytest-django.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
.vscode/settings.jsonsrc/core/forms.pysrc/core/templatetags/__init__.pysrc/core/templatetags/media_tags.pysrc/core/urls.pysrc/core/views.pysrc/templates/backup_manage.htmlsrc/templates/partials/media-contributors.htmlsrc/templates/partials/media-cover.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-icon.htmlsrc/templates/partials/media-items.htmlsrc/templates/partials/media-list.htmlsrc/templates/partials/media-review-clamped.htmlsrc/templates/partials/media-review-full.htmlsrc/templates/partials/media-score-badge.htmlsrc/templates/partials/media-status-badge.htmlsrc/templates/partials/review-date-editable.htmlsrc/templates/partials/score-editable.htmlsrc/templates/partials/status-editable.htmlsrc/tests/core/test_views.py
💤 Files with no reviewable changes (6)
- src/templates/partials/score-editable.html
- src/core/urls.py
- src/templates/partials/status-editable.html
- src/templates/backup_manage.html
- src/templates/partials/review-date-editable.html
- src/core/views.py
🚧 Files skipped from review as they are similar to previous changes (10)
- src/templates/partials/media-cover.html
- src/templates/partials/media-score-badge.html
- .vscode/settings.json
- src/templates/partials/media-contributors.html
- src/templates/partials/media-review-full.html
- src/templates/partials/media-status-badge.html
- src/core/templatetags/media_tags.py
- src/core/forms.py
- src/templates/partials/media-icon.html
- src/templates/partials/media-list.html
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-01-03T21:16:52.649Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-items.html:67-67
Timestamp: 2026-01-03T21:16:52.649Z
Learning: In HTML templates (e.g., src/templates/partials/media-items.html), Tailwind CSS v4.1.11 supports arbitrary numeric utilities (max-w-{n}, w-{n}, h-{n}) that map to calc(var(--spacing) * n). These should be considered valid Tailwind classes and not flagged as invalid. Ensure review tooling and linters treat such numeric utilities as allowed, and apply this guidance to other HTML/template files in the repository.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-review-clamped.html
📚 Learning: 2026-01-04T08:55:43.091Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 36
File: src/templates/partials/media-score-badge.html:1-9
Timestamp: 2026-01-04T08:55:43.091Z
Learning: In Django projects, configuring heroicons in TEMPLATES builtins makes heroicons.templatetags.heroicons available in all templates without needing {% load heroicons %}. For template reviews in this codebase, assume heroicon tags work globally in HTML templates under templates/, so avoid requiring explicit loads. If a template still uses {% load heroicons %}, assess whether the load is unnecessary and can be removed; ensure no conflicts arise from global tags.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-review-clamped.html
📚 Learning: 2025-12-26T15:18:46.932Z
Learnt from: PascalRepond
Repo: PascalRepond/datakult PR: 21
File: src/templates/accounts/profile_edit.html:23-58
Timestamp: 2025-12-26T15:18:46.932Z
Learning: In Django projects, attributes added to a form field's widget via field.widget.attrs.update(...) in the form's __init__ are rendered when using {{ form.field }} in templates. No explicit attribute definitions are needed in the template. This applies to templates under src/templates in Django apps; ensure you update attrs in __init__ for consistent HTMX behavior.
Applied to files:
src/templates/partials/media-items.htmlsrc/templates/partials/media-edit-button.htmlsrc/templates/partials/media-review-clamped.html
🧬 Code graph analysis (2)
src/templates/partials/media-items.html (3)
src/core/migrations/0003_media_review_media_review_date_media_score_and_more.py (1)
Migration(9-41)src/core/migrations/0008_media_review_rendered_alter_media_review.py (1)
Migration(24-43)src/core/migrations/0005_media_cover_alter_media_score.py (1)
Migration(6-23)
src/templates/partials/media-edit-button.html (1)
src/core/views.py (1)
media_edit(135-170)
🪛 Ruff (0.14.10)
src/tests/core/test_views.py
359-359: Unused method argument: db
(ARG002)
🔇 Additional comments (7)
src/templates/partials/media-review-clamped.html (2)
3-4: LGTM! Threshold increase improves readability.The truncation threshold has been increased from 20 to 50 words, and the conditional check is consistently updated. This allows users to read more content before requiring the "See more" button, which enhances the user experience.
5-5: LGTM! Button styling updated for better visual hierarchy.The button styling has been updated from
btn-ghosttobtn-outlineand the margin adjusted frommt-1to-mt-3, which improves the visual integration with the review content.src/tests/core/test_views.py (2)
356-357: LGTM! Comment now accurately reflects the 50-word threshold.The comment has been updated to correctly state "less than 50 words", addressing the past review feedback. This improves test documentation clarity.
361-361: LGTM! Test boundaries updated to match new 50-word threshold.The test now correctly creates a 55-word review (above the 50-word threshold) and the comment accurately describes the expected behavior. These changes align perfectly with the template modifications in
media-review-clamped.html.Also applies to: 370-371
src/templates/partials/media-edit-button.html (1)
1-7: LGTM! Clean, accessible edit button partial.The implementation follows good practices:
- Proper i18n support with
{% translate %}for both tooltip and aria-label- Accessible: combines translated "Edit" with media title in aria-label for screen readers
- Uses DaisyUI's tooltip pattern correctly
- Heroicon works via global builtins config (no explicit load needed per learnings)
src/templates/partials/media-items.html (2)
1-43: Well-structured grid view with good accessibility.The refactoring to use modular partials improves maintainability. Key improvements noted:
- Proper accessibility with
aria-label="{{ media.get_media_type_display }}"on the media type badge- Translated alt text for cover images
- Clean conditional rendering for optional elements (score, review)
- Heroicon tag works via global builtins config (per learnings)
44-85: Clean list view implementation with responsive design.The table-based list view is well-organized:
- Responsive column visibility (
hidden md:table-cell, etc.) provides good mobile experience- Consistent cell styling with
align-top p-2- Proper fallback for missing review_date with em dash
max-w-200is valid Tailwind v4 numeric utility (per learnings)- Modular partial usage matches grid view pattern