lukas/timetracker
1
0
Fork 0
Code Issues 17 Pull Requests Actions Packages Projects 1 Releases 23 Wiki Activity

Make component return types honest; drop str/mark_safe leftovers

Browse Source 
Cleanup of hacky leftovers from the node-tree migration (no behaviour
change):

- Return annotations: the component builders return Node subtrees, not
  SafeText strings, but ~40 functions still declared `-> SafeText`. Correct
  them to `-> Node` across filters / search_select / date_range_picker /
  domain. The genuine string returners keep `-> SafeText`: the Alpine
  selectors (GameStatusSelector / SessionDeviceSelector, which build f-string
  markup) and the script-tag helpers (CsrfInput / ModuleScript /
  ExternalScript / StaticScript).
- layout.render_page / layout.Page / AddForm now accept `Node` in their
  `content` / `scripts` / `fields` parameters (TYPE_CHECKING import in
  layout to avoid the components import cycle), matching what views already
  pass.
- session._session_fields builds a `Fragment(*rows, separator="\n")` instead
  of `mark_safe("\n".join(str(row) ...))` — keeps the tree intact so media
  could bubble, per the Fragment convention.
- Inline SVG icon children use `Safe(...)` nodes instead of `mark_safe(...)`
  strings (filters mode-toggle + collapse icons, date_range_picker calendar
  icon).
- _filter_field reads the widget's own id from its node `.attributes`
  (`_widget_id`) for the label's `for`, dropping the superfluous `for_widget`
  argument that always rendered `for="None"`. Removes the two TODOs whose
  premise ("the Component function can't expose the id") the class/node
  refactor retired, plus RangeSlider's dead commented-out Label block.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Lukáš Kucharczyk
2026-06-13 15:12:52 +02:00
parent 1c5bff8651
commit 022d43a5a5
7 changed files with 86 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files
common/components/date_range_picker.py
+9 -10
View File
@@ -17,9 +17,8 @@ widget into a ``DateCriterion`` unchanged. All behaviour is wired by
``games/static/js/date_range_picker.js``.
"""
from django.utils.safestring import SafeText, mark_safe
from common.components.core import Element, HTMLAttribute, Media, Node
from common.components.core import Element, HTMLAttribute, Media, Node, Safe
from common.components.primitives import Div, Input, Span
from common.time import DatePartSpec, date_parts
@@ -104,7 +103,7 @@ def _iso_part_values(iso_value: str, parts: list[DatePartSpec]) -> dict[str, str
def _segment_input(
*, part: DatePartSpec, side: str, label: str, value: str
) -> SafeText:
) -> Node:
side_label = "from" if side == "min" else "to"
return Input(
attributes=[
@@ -125,11 +124,11 @@ def _segment_input(
)
def _segment_group(*, side: str, label: str, iso_value: str) -> SafeText:
def _segment_group(*, side: str, label: str, iso_value: str) -> Node:
"""One date's worth of segments (``DD - MM - YYYY``) for a range side."""
parts = date_parts()
initial_values = _iso_part_values(iso_value, parts)
children: list[SafeText] = []
children: list[Node] = []
for index, part in enumerate(parts):
if index > 0:
children.append(
@@ -161,7 +160,7 @@ def DateRangeField(
input_name_prefix: str,
min_value: str = "",
max_value: str = "",
) -> SafeText:
) -> Node:
"""The visible half of the DateRangePicker: a single-input-looking
container holding two segmented dates, a calendar toggle, and the two
hidden ISO inputs (``{prefix}-min`` / ``{prefix}-max``) that carry the
@@ -210,13 +209,13 @@ def DateRangeField(
"cursor-pointer shrink-0",
),
],
children=[mark_safe(_CALENDAR_ICON_SVG)],
children=[Safe(_CALENDAR_ICON_SVG)],
),
],
)
def _calendar_nav_button(direction: str, arrow: str, label: str) -> SafeText:
def _calendar_nav_button(direction: str, arrow: str, label: str) -> Node:
return Element(
"button",
attributes=[
@@ -229,7 +228,7 @@ def _calendar_nav_button(direction: str, arrow: str, label: str) -> SafeText:
)
def _footer_button(action: str, label: str, button_class: str) -> SafeText:
def _footer_button(action: str, label: str, button_class: str) -> Node:
return Element(
"button",
attributes=[
@@ -241,7 +240,7 @@ def _footer_button(action: str, label: str, button_class: str) -> SafeText:
)
def DateRangeCalendar(*, input_name_prefix: str) -> SafeText:
def DateRangeCalendar(*, input_name_prefix: str) -> Node:
"""The popup half of the DateRangePicker: preset column, month grid
(filled client-side into ``[data-date-range-grid]``), and the
Cancel / Clear / Select footer. Hidden until the calendar toggle opens it."""
common/components/domain.py
+7 -7
View File
@@ -6,7 +6,7 @@ from django.template.defaultfilters import floatformat
from django.urls import reverse
from django.utils.safestring import SafeText, mark_safe
from common.components.core import HTMLTag
from common.components.core import HTMLTag, Node
from common.components.primitives import (
A,
Div,
@@ -22,7 +22,7 @@ def GameLink(
game_id: int,
name: str = "",
children: list[HTMLTag] | HTMLTag | None = None,
) -> SafeText:
) -> Node:
"""Link to a game's detail page. Uses children (slot) if provided, otherwise name."""
from django.urls import reverse
@@ -58,7 +58,7 @@ def GameStatus(
status: str = "u",
display: str = "",
class_: str = "",
) -> SafeText:
) -> Node:
"""Colored status dot with label. Status codes: u/p/f/a/r."""
children = children or []
outer_class = (
@@ -82,7 +82,7 @@ def GameStatus(
def PriceConverted(
children: list[HTMLTag] | HTMLTag | None = None,
) -> SafeText:
) -> Node:
"""Wrap content in a span that indicates the price was converted."""
children = children or []
return Span(
@@ -94,7 +94,7 @@ def PriceConverted(
)
def LinkedPurchase(purchase: Purchase) -> SafeText:
def LinkedPurchase(purchase: Purchase) -> Node:
link = reverse("games:view_purchase", args=[int(purchase.id)])
link_content = ""
popover_content = ""
@@ -145,7 +145,7 @@ def NameWithIcon(
session: Session | None = None,
linkify: bool = True,
emulated: bool = False,
) -> SafeText:
) -> Node:
_name, platform, final_emulated, create_link, link = _resolve_name_with_icon(
name, game, session, linkify
)
@@ -203,7 +203,7 @@ def _resolve_name_with_icon(
return _name, platform, final_emulated, create_link, link
def PurchasePrice(purchase) -> SafeText:
def PurchasePrice(purchase) -> Node:
return Popover(
popover_content=f"{floatformat(purchase.price)} {purchase.price_currency}",
wrapped_content=f"{floatformat(purchase.converted_price)} {purchase.converted_currency}",
common/components/filters.py
+38 -35
View File
@@ -3,9 +3,8 @@
from typing import NamedTuple
from django.db import models
from django.utils.safestring import SafeText, mark_safe
from common.components.core import BaseComponent, Element, Media, Node
from common.components.core import BaseComponent, Element, Media, Node, Safe
from common.components.date_range_picker import DateRangePicker
from common.components.primitives import Checkbox, Div, Input, Label, Radio, Span
from common.components.search_select import (
@@ -176,7 +175,7 @@ def _split_modifier(modifier: str, has_m2m: bool = False) -> str:
def _enum_filter(
field_name: str, options, choice: FilterChoice, *, nullable
) -> SafeText:
) -> Node:
"""A FilterSelect over a small, fully pre-rendered option set (enum field).
Enum fields are single-valued, so no M2M modifiers (all/only are
@@ -207,7 +206,7 @@ def _model_filter(
search_url,
nullable,
m2m_modifiers: list[LabeledOption] | None = None,
) -> SafeText:
) -> Node:
"""A FilterSelect backed by a search endpoint.
Labels are embedded in the filter JSON (Stash-style), so pills render
@@ -240,34 +239,43 @@ def _filter_mins_to_hrs(val) -> str:
return str(int(hrs)) if hrs == int(hrs) else f"{hrs:.1f}"
def _filter_field(label: str, widget, for_widget: str = None) -> SafeText:
"""A labelled filter field: <div><label>…</label>{widget}</div>.
TODO: Use widget.attributes.get("id", "") to get the widget's ID
instead of the superfluous "for" argument. This requires refactoring
the Component function to be a class intead.
Also see RangeSlider's TODO
def _widget_id(widget) -> str:
"""Best-effort id of a widget node, for the field label's ``for`` target.
Widgets are nodes carrying ``.attributes``, so the id is now reachable
directly (the old free ``Component`` string couldn't expose it).
"""
for name, value in getattr(widget, "attributes", []):
if name == "id":
return str(value)
return ""
def _filter_field(label: str, widget) -> Node:
"""A labelled filter field: ``<div><label>…</label>{widget}</div>``.
The label's ``for`` points at the widget's own id when it has one;
composite widgets without a single root id simply omit ``for``.
"""
label_attributes = [("class", _FILTER_LABEL_CLASS)]
widget_id = _widget_id(widget)
if widget_id:
label_attributes.append(("for", widget_id))
return Div(
attributes=[("class", "flex flex-col gap-1")],
children=[
Label(
attributes=[
("class", _FILTER_LABEL_CLASS),
("for", for_widget),
],
children=[label],
),
Label(attributes=label_attributes, children=[label]),
widget,
],
)
def _filter_checkbox(name: str, label: str, checked: bool) -> SafeText:
def _filter_checkbox(name: str, label: str, checked: bool) -> Node:
"""Thin adapter mapping legacy checkbox filters to the generalized Checkbox primitive."""
return Checkbox(name=name, label=label, checked=checked)
def _filter_boolean_radio(name: str, label: str, value: bool | None) -> SafeText:
def _filter_boolean_radio(name: str, label: str, value: bool | None) -> Node:
"""Renders a filter-specific boolean radio button group with 'True' and 'False' options."""
return Div(
attributes=[("class", "flex flex-col gap-1")],
@@ -321,7 +329,7 @@ def RangeSlider(
step: str = "1",
min_placeholder: str = "",
max_placeholder: str = "",
) -> SafeText:
) -> Node:
"""A labelled range slider with number inputs and range/point mode toggle.
Renders a label row (label, two number inputs, toggle button) and a slider
@@ -341,14 +349,9 @@ def RangeSlider(
Div(
attributes=[("class", "flex items-center gap-2 mb-1")],
children=[
# TODO: This should be done outside the RangeSlider component, but the current Component function doesn't allow getting the id
# Label(
# attributes=[
# ("class", _FILTER_LABEL_CLASS),
# ("for", min_input_id),
# ],
# children=[label],
# ),
# The field label is rendered by the _filter_field wrapper.
# This composite widget has no single labelable root, so the
# label carries no `for` (the two inputs are named below).
Input(
attributes=[
("type", "number"),
@@ -410,7 +413,7 @@ def RangeSlider(
+ (" hidden" if point_mode else ""),
),
],
children=[mark_safe(_RANGE_ICON_SVG)],
children=[Safe(_RANGE_ICON_SVG)],
),
Span(
attributes=[
@@ -420,7 +423,7 @@ def RangeSlider(
+ ("" if point_mode else " hidden"),
),
],
children=[mark_safe(_POINT_ICON_SVG)],
children=[Safe(_POINT_ICON_SVG)],
),
],
),
@@ -506,7 +509,7 @@ def DateRangeFilter(
max_value: str = "",
min_placeholder: str = "From",
max_placeholder: str = "To",
) -> SafeText:
) -> Node:
"""A pair of ``<input type="date">`` elements representing a date range.
Mirrors ``RangeSlider`` in shape (two inputs named ``{prefix}-min`` and
@@ -561,7 +564,7 @@ _FILTER_FORM_ID = "filter-bar-form"
_FILTER_INPUT_ID = "filter-json-input"
def _filter_collapse_button() -> SafeText:
def _filter_collapse_button() -> Node:
return Element(
"button",
attributes=[
@@ -579,7 +582,7 @@ def _filter_collapse_button() -> SafeText:
),
],
children=[
mark_safe(
Safe(
'<svg class="w-4 h-4" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor"><path stroke-linecap="round" stroke-linejoin="round" d="M10.5 6h9.75M10.5 6a1.5 1.5 0 1 1-3 0m3 0a1.5 1.5 0 1 0-3 0M3.75 6H7.5m3 12h9.75m-9.75 0a1.5 1.5 0 0 1-3 0m3 0a1.5 1.5 0 0 0-3 0m-3.75 0H7.5m9-6h3.75m-3.75 0a1.5 1.5 0 0 1-3 0m3 0a1.5 1.5 0 0 0-3 0m-9.75 0h9.75" /></svg>'
),
"Filters",
@@ -587,7 +590,7 @@ def _filter_collapse_button() -> SafeText:
)
def _filter_action_row(preset_list_url: str, preset_save_url: str) -> SafeText:
def _filter_action_row(preset_list_url: str, preset_save_url: str) -> Node:
return Div(
attributes=[("class", "flex gap-3 items-center")],
children=[
@@ -1529,7 +1532,7 @@ def StringFilter(
value: str = "",
modifier: str = "EQUALS",
placeholder: str = "",
) -> SafeText:
) -> Node:
"""Renders a string filter with 8 modifier radio options and a text input."""
from common.criteria import Modifier
common/components/primitives.py
+2 -2
View File
@@ -622,8 +622,8 @@ def AddForm(
form,
*,
request,
fields: SafeText | str | None = None,
additional_row: SafeText | str = "",
fields: Node | SafeText | str | None = None,
additional_row: Node | SafeText | str = "",
submit_class: str = "mt-3",
) -> Node:
"""Page body for the generic add/edit form (Python equivalent of add.html).
common/components/search_select.py
+18 -19
View File
@@ -21,7 +21,6 @@ user types.
from collections.abc import Callable, Iterable
from typing import TypedDict
from django.utils.safestring import SafeText
from common.components.core import Element, HTMLAttribute, Media, Node
from common.components.primitives import Div, Input, Pill, Span, Template
@@ -144,11 +143,11 @@ def _data_attributes(data: dict[str, str]) -> list[HTMLAttribute]:
return [(f"data-{key}", str(value)) for key, value in data.items()]
def _hidden_input(name: str, value) -> SafeText:
def _hidden_input(name: str, value) -> Node:
return Input(type="hidden", attributes=[("name", name), ("value", str(value))])
def _label_slot(text: str, *, extra_class: str = "") -> SafeText:
def _label_slot(text: str, *, extra_class: str = "") -> Node:
"""A ``<span data-search-select-label>`` holding a row/pill's visible label. JS fills this
one node when cloning the shape from a ``<template>``, so labels are the only
thing the JS sets — all classes and structure stay server-side."""
@@ -162,7 +161,7 @@ def _label_slot(text: str, *, extra_class: str = "") -> SafeText:
_BLANK_OPTION: SearchSelectOption = {"value": "", "label": "", "data": {}}
def _option_row(option: SearchSelectOption) -> SafeText:
def _option_row(option: SearchSelectOption) -> Node:
return Div(
attributes=[
("data-search-select-option", ""),
@@ -178,13 +177,13 @@ def _option_row(option: SearchSelectOption) -> SafeText:
def _combobox_shell(
*,
container_attributes: list[HTMLAttribute],
pills: SafeText,
pills: Node,
search_attributes: list[HTMLAttribute],
options_children: list[SafeText],
options_children: list[Node],
always_visible: bool,
items_visible: int,
templates: list[SafeText] | None = None,
) -> SafeText:
templates: list[Node] | None = None,
) -> Node:
"""Assemble the shared, domain-agnostic combobox skeleton.
Every combobox built on top of this shell has the same three regions in the
@@ -216,7 +215,7 @@ def _combobox_shell(
children=[*options_children, no_results],
)
children: list[SafeText] = [pills, search, options_panel, *(templates or [])]
children: list[Node] = [pills, search, options_panel, *(templates or [])]
return Div(attributes=container_attributes, children=children)
@@ -235,7 +234,7 @@ def SearchSelect(
id: str = "",
sync_url: bool = False,
autofocus: bool = False,
) -> SafeText:
) -> Node:
"""Render the search-select widget. See module docstring for the contract."""
selected = [_normalize_option(option) for option in (selected or [])]
options = [_normalize_option(option) for option in (options or [])]
@@ -245,7 +244,7 @@ def SearchSelect(
# pill — the committed label shows inside the search box instead, with a
# lone hidden input carrying the value. Both keep the hidden input(s) inside
# `[data-search-select-pills]` so the JS reads/writes values uniformly.
pills_children: list[SafeText] = []
pills_children: list[Node] = []
search_value = ""
if multi_select:
for option in selected:
@@ -286,7 +285,7 @@ def SearchSelect(
# ── Templates the JS clones: a row when results are fetched, a pill when
# multi-select adds chosen items. ──
templates: list[SafeText] = []
templates: list[Node] = []
if search_url:
templates.append(
Template(
@@ -341,7 +340,7 @@ def _filter_remove_button() -> Node:
)
def _filter_value_pill(option: SearchSelectOption, kind: str) -> SafeText:
def _filter_value_pill(option: SearchSelectOption, kind: str) -> Node:
"""An include (✓) or exclude (✗) value pill. ``kind`` is "include"/"exclude"."""
symbol = "✓" if kind == "include" else "✗"
css = (
@@ -360,7 +359,7 @@ def _filter_value_pill(option: SearchSelectOption, kind: str) -> SafeText:
)
def _filter_modifier_pill(modifier_value: str, label: str) -> SafeText:
def _filter_modifier_pill(modifier_value: str, label: str) -> Node:
"""The lone, sticky modifier pill (e.g. "(Any)"/"(None)")."""
return Span(
attributes=[
@@ -385,7 +384,7 @@ def _filter_action_button(action: str, symbol: str, title: str) -> Node:
)
def _filter_option_row(value: str | int, label: str) -> SafeText:
def _filter_option_row(value: str | int, label: str) -> Node:
"""A value row with include (+) and exclude (−) buttons."""
return Div(
attributes=[
@@ -407,7 +406,7 @@ def _filter_option_row(value: str | int, label: str) -> SafeText:
)
def _filter_modifier_row(modifier_value: str, label: str) -> SafeText:
def _filter_modifier_row(modifier_value: str, label: str) -> Node:
"""A pinned pseudo-option row. It carries no ``data-search-select-option`` so the text
filter never hides it — modifiers stay visible at the top of the panel."""
return Div(
@@ -435,7 +434,7 @@ def FilterSelect(
placeholder: str = "Search…",
id: str = "",
free_text: bool = False,
) -> SafeText:
) -> Node:
"""Include/exclude filter combobox built on the shared ``_combobox_shell``.
Like ``SearchSelect`` but each value row carries +/− buttons that add an
@@ -473,7 +472,7 @@ def FilterSelect(
# pills — but the stored state guarantees they never coexist, so we render
# both channels unconditionally. Non-presence modifiers (INCLUDES_ALL /
# INCLUDES_ONLY) coexist with value pills and render side by side.
pills_children: list[SafeText] = []
pills_children: list[Node] = []
if active_modifier_label:
pills_children.append(_filter_modifier_pill(modifier, active_modifier_label))
for option in included:
@@ -507,7 +506,7 @@ def FilterSelect(
# ── Templates the JS clones: include/exclude pills (added on click), the
# modifier pill (when modifiers exist), and a value row (when fetched). ──
templates: list[SafeText] = [
templates: list[Node] = [
Template(
attributes=[("data-search-select-template", "pill-include")],
children=[_filter_value_pill(_BLANK_OPTION, "include")],
common/layout.py
+8 -4
View File
@@ -8,6 +8,7 @@ it hoists shared `<head>` content (the `_HEADERS` block, analogous to
"""
import json
from typing import TYPE_CHECKING
from django.contrib.messages import get_messages
from django.http import HttpRequest, HttpResponse
@@ -19,6 +20,9 @@ from django_htmx.jinja import django_htmx_script
from games.templatetags.version import version, version_date
if TYPE_CHECKING:
from common.components import Node
# Static head script that sets the dark/light class before paint (avoids FOUC).
_THEME_FOUC_SCRIPT = """<script>
if (localStorage.getItem('color-theme') === 'dark' || (!('color-theme' in localStorage) && window.matchMedia('(prefers-color-scheme: dark)').matches)) {
@@ -269,11 +273,11 @@ def Navbar(*, today_played: str, last_7_played: str, current_year: int) -> SafeT
def Page(
content: SafeText | str,
content: "Node | SafeText | str",
*,
request: HttpRequest,
title: str = "",
scripts: SafeText | str = "",
scripts: "Node | SafeText | str" = "",
mastered: bool = False,
) -> SafeText:
"""Assemble a full HTML document around `content` (the fast_app equivalent).
@@ -356,10 +360,10 @@ def Page(
def render_page(
request: HttpRequest,
content: SafeText | str,
content: "Node | SafeText | str",
*,
title: str = "",
scripts: SafeText | str = "",
scripts: "Node | SafeText | str" = "",
mastered: bool = False,
status: int = 200,
) -> HttpResponse: