From a1834a1c30b8434806cb6677e596571c4fdc887b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:15:47 +0200 Subject: [PATCH 01/15] feat: four duration rendering profiles Decimal hours, hours and minutes, whole hours, and an adaptive ladder from minutes through years. Every profile rounds the total once and then decomposes, which is what keeps 1 h 59 m 45 s from rendering as 2 h 60 m and 6 d 23 h 40 m from rendering as 6 d 24 h. Co-Authored-By: Claude Opus 5 --- common/duration_presentation.py | 220 ++++++++++++++++++++++++++++ tests/test_duration_presentation.py | 110 ++++++++++++++ 2 files changed, 330 insertions(+) create mode 100644 common/duration_presentation.py create mode 100644 tests/test_duration_presentation.py diff --git a/common/duration_presentation.py b/common/duration_presentation.py new file mode 100644 index 00000000..3313438b --- /dev/null +++ b/common/duration_presentation.py @@ -0,0 +1,220 @@ +"""Request-scoped formatting for every human-visible elapsed duration. + +A duration is not a datetime: it has no timezone, no calendar, and no 12/24-hour +clock, so it carries its own preference rather than riding on ``DATETIME_FORMAT``. +Stored values, API values, filtering, and sorting never come through here — this +module is display only. + +Two rules hold across every profile: + +* **Round the total once, then decompose.** Rounding a component after the split + renders 1 h 59 m 45 s as "2 h 60 m". +* **Round, never truncate**, at each profile's own resolution. Truncation renders + 59 minutes as "0 hours", which is a lie the popover should not have to repair. +""" + +import math +from dataclasses import dataclass +from datetime import timedelta +from functools import cache +from types import MappingProxyType +from typing import Protocol + +from django.http import HttpRequest + +from timetracker.settings_resolver import resolve_str_for_user + +type DurationProfileId = str # e.g. "decimal_hours" +type ProfileLabel = str # e.g. "Hours and minutes" + +MINUTE_SECONDS = 60 +HOUR_SECONDS = 60 * MINUTE_SECONDS +DAY_SECONDS = 24 * HOUR_SECONDS +WEEK_SECONDS = 7 * DAY_SECONDS +# A year is not a whole number of weeks. 52 weeks keeps the ladder exact, and +# nothing here is a calendar date, so the four-day drift is harmless. +YEAR_SECONDS = 52 * WEEK_SECONDS + + +class NumberFormatter(Protocol): + """Renders a number for display. Locale-aware in a request, plain in tests + of the rules themselves.""" + + def __call__(self, value: float, decimals: int) -> str: ... + + +def plain_number(value: float, decimals: int) -> str: + return f"{value:.{decimals}f}" + + +def _round_half_away(value: float) -> int: + """Half away from zero, unlike ``round()``'s banker's rounding. Durations + are clamped non-negative before they reach here.""" + return math.floor(value + 0.5) + + +def _total_seconds(duration: timedelta | float | None) -> int: + """Whole seconds, never negative: ``timestamp_end`` before + ``timestamp_start`` is bad data, not a negative duration.""" + if duration is None: + return 0 + seconds = ( + duration.total_seconds() if isinstance(duration, timedelta) else float(duration) + ) + return max(int(seconds), 0) + + +@dataclass(frozen=True) +class LadderUnit: + """One rung of the adaptive ladder.""" + + key: str # e.g. "day" + seconds: int + symbol: str # e.g. "d" + pad_below: bool # zero-pad the next unit down to two digits + + +# Largest first; the search below relies on the order. +LADDER: tuple[LadderUnit, ...] = ( + LadderUnit("year", YEAR_SECONDS, "y", pad_below=False), + LadderUnit("week", WEEK_SECONDS, "w", pad_below=False), + LadderUnit("day", DAY_SECONDS, "d", pad_below=True), + LadderUnit("hour", HOUR_SECONDS, "h", pad_below=True), + LadderUnit("minute", MINUTE_SECONDS, "m", pad_below=False), +) + + +@dataclass(frozen=True) +class DurationProfile: + """One display rendering. ``label`` names it in the settings control and in + the popover listing the other profiles.""" + + id: DurationProfileId + label: ProfileLabel + + def render(self, seconds: int, numbers: NumberFormatter) -> str: + raise NotImplementedError + + +@dataclass(frozen=True) +class DecimalHoursProfile(DurationProfile): + def render(self, seconds: int, numbers: NumberFormatter) -> str: + tenths = _round_half_away(seconds / HOUR_SECONDS * 10) + return f"{numbers(tenths / 10, 1)} h" + + +@dataclass(frozen=True) +class HoursMinutesProfile(DurationProfile): + def render(self, seconds: int, numbers: NumberFormatter) -> str: + total_minutes = _round_half_away(seconds / MINUTE_SECONDS) + hours, minutes = divmod(total_minutes, 60) + if hours: + return f"{numbers(hours, 0)} h {minutes:02d} m" + # Zero keeps the column's unit stable; anything else under an hour drops + # the pointless leading "0 h". + return f"{minutes} m" if minutes else "0 h" + + +@dataclass(frozen=True) +class WholeHoursProfile(DurationProfile): + def render(self, seconds: int, numbers: NumberFormatter) -> str: + hours = _round_half_away(seconds / HOUR_SECONDS) + return f"{numbers(hours, 0)} hour{'' if hours == 1 else 's'}" + + +@dataclass(frozen=True) +class AdaptiveProfile(DurationProfile): + """Minutes through years, showing the largest unit the value reaches and + the one below it.""" + + def render(self, seconds: int, numbers: NumberFormatter) -> str: + unit, below = self._units_for(seconds) + if below is None: + return self._minutes_only(seconds, numbers) + rounded = _round_half_away(seconds / below.seconds) * below.seconds + # Rounding can carry into a higher unit — 6 d 23 h 40 m becomes 1 w 0 d, + # never 6 d 24 h. + unit, below = self._units_for(rounded) + if below is None: + return self._minutes_only(rounded, numbers) + top, remainder = divmod(rounded, unit.seconds) + lower = remainder // below.seconds + lower_text = f"{lower:02d}" if unit.pad_below else str(lower) + return f"{numbers(top, 0)} {unit.symbol} {lower_text} {below.symbol}" + + def _minutes_only(self, seconds: int, numbers: NumberFormatter) -> str: + """Below an hour the ladder agrees with hours_minutes, zero included: + an empty duration reads "0 h" so a column's unit stays put.""" + minutes = _round_half_away(seconds / MINUTE_SECONDS) + return f"{numbers(minutes, 0)} m" if minutes else "0 h" + + def _units_for(self, seconds: int) -> tuple[LadderUnit, LadderUnit | None]: + """The largest unit ``seconds`` reaches and the rung below it. Below a + minute the pair collapses to minutes alone.""" + for index, unit in enumerate(LADDER): + if seconds >= unit.seconds: + below = LADDER[index + 1] if index + 1 < len(LADDER) else None + return unit, below + return LADDER[-1], None + + +DURATION_FORMAT_PROFILES = MappingProxyType( + { + "decimal_hours": DecimalHoursProfile("decimal_hours", "Decimal hours"), + "hours_minutes": HoursMinutesProfile("hours_minutes", "Hours and minutes"), + "whole_hours": WholeHoursProfile("whole_hours", "Whole hours"), + "adaptive": AdaptiveProfile("adaptive", "Adaptive units"), + } +) + +DEFAULT_DURATION_FORMAT_PROFILE = DURATION_FORMAT_PROFILES["decimal_hours"] + + +def duration_format_profile(profile_id: DurationProfileId) -> DurationProfile: + """Return the registered immutable profile for ``profile_id``.""" + try: + return DURATION_FORMAT_PROFILES[profile_id] + except KeyError as error: + raise ValueError(f"Unsupported duration format {profile_id!r}.") from error + + +def format_decimal_hours(duration: timedelta | float | None) -> str: + """Decimal hours, preference-independent, for callers with no request — + ``Session.__str__`` and other debug or log strings.""" + seconds = _total_seconds(duration) + return plain_number(_round_half_away(seconds / HOUR_SECONDS * 10) / 10, 1) + + +@dataclass(frozen=True) +class DurationPresentation: + """The immutable duration display contract active for one request.""" + + profile: DurationProfile + locale: str + + def format(self, duration: timedelta | float | None) -> str: + return self.profile.render(_total_seconds(duration), plain_number) + + +_REQUEST_CACHE_ATTRIBUTE = "_duration_presentation" + + +@cache +def _profile_for_id(profile_id: DurationProfileId) -> DurationProfile: + return duration_format_profile(profile_id) + + +def duration_presentation_for_request(request: HttpRequest) -> DurationPresentation: + """Resolve and cache the presentation directly on ``request``.""" + cached = getattr(request, _REQUEST_CACHE_ATTRIBUTE, None) + if isinstance(cached, DurationPresentation): + return cached + + profile_id = resolve_str_for_user(getattr(request, "user", None), "DURATION_FORMAT") + locale = getattr(request, "_date_format_locale", None) + presentation = DurationPresentation( + profile=_profile_for_id(profile_id), + locale=locale if isinstance(locale, str) else "en-us", + ) + setattr(request, _REQUEST_CACHE_ATTRIBUTE, presentation) + return presentation diff --git a/tests/test_duration_presentation.py b/tests/test_duration_presentation.py new file mode 100644 index 00000000..14f266ec --- /dev/null +++ b/tests/test_duration_presentation.py @@ -0,0 +1,110 @@ +"""Every duration profile's exact rendering, including the carry boundaries. + +The table mirrors the rendering rules in +docs/superpowers/specs/2026-07-29-issue-486-duration-format-design.md — it is +the specification, so a change here is a design change, not a test fix. +""" + +from datetime import timedelta + +import pytest + +from common.duration_presentation import ( + DurationPresentation, + duration_format_profile, + format_decimal_hours, +) + +MINUTE = 60 +HOUR = 60 * MINUTE +DAY = 24 * HOUR + +# (seconds, decimal_hours, hours_minutes, whole_hours, adaptive) +RENDERINGS = [ + (0, "0.0 h", "0 h", "0 hours", "0 h"), + (45, "0.0 h", "1 m", "0 hours", "1 m"), + (29 * MINUTE, "0.5 h", "29 m", "0 hours", "29 m"), + (45 * MINUTE, "0.8 h", "45 m", "1 hour", "45 m"), + (HOUR + 12 * MINUTE, "1.2 h", "1 h 12 m", "1 hour", "1 h 12 m"), + (3 * HOUR + 5 * MINUTE, "3.1 h", "3 h 05 m", "3 hours", "3 h 05 m"), + (3 * HOUR + 30 * MINUTE, "3.5 h", "3 h 30 m", "4 hours", "3 h 30 m"), + (23 * HOUR + 59 * MINUTE + 45, "24.0 h", "24 h 00 m", "24 hours", "1 d 00 h"), + (26 * HOUR, "26.0 h", "26 h 00 m", "26 hours", "1 d 02 h"), + (83 * HOUR + 12 * MINUTE, "83.2 h", "83 h 12 m", "83 hours", "3 d 11 h"), + (200 * HOUR, "200.0 h", "200 h 00 m", "200 hours", "1 w 1 d"), + (1234 * HOUR, "1234.0 h", "1234 h 00 m", "1234 hours", "7 w 2 d"), + (9000 * HOUR, "9000.0 h", "9000 h 00 m", "9000 hours", "1 y 2 w"), +] + +PROFILE_IDS = ("decimal_hours", "hours_minutes", "whole_hours", "adaptive") + + +def _presentation(profile_id: str) -> DurationPresentation: + return DurationPresentation( + profile=duration_format_profile(profile_id), locale="en-us" + ) + + +@pytest.mark.parametrize("row", RENDERINGS, ids=[str(row[0]) for row in RENDERINGS]) +@pytest.mark.parametrize("index,profile_id", list(enumerate(PROFILE_IDS, start=1))) +def test_renders_the_specified_value(row, index, profile_id): + seconds, *expected_per_profile = row + + rendered = _presentation(profile_id).format(timedelta(seconds=seconds)) + + assert rendered == expected_per_profile[index - 1] + + +@pytest.mark.parametrize("profile_id", PROFILE_IDS) +def test_none_renders_as_zero(profile_id): + presentation = _presentation(profile_id) + + assert presentation.format(None) == presentation.format(timedelta(0)) + + +@pytest.mark.parametrize("profile_id", PROFILE_IDS) +def test_negative_clamps_to_zero(profile_id): + """timestamp_end before timestamp_start is bad data, not a negative + duration.""" + presentation = _presentation(profile_id) + + assert presentation.format(timedelta(seconds=-500)) == presentation.format( + timedelta(0) + ) + + +@pytest.mark.parametrize( + "seconds,expected", + [ + # Rounding a component after the split would give "2 h 60 m". + (HOUR + 59 * MINUTE + 45, "2 h 00 m"), + (29 * MINUTE + 30, "30 m"), + ], +) +def test_hours_minutes_rounds_the_total_once(seconds, expected): + assert _presentation("hours_minutes").format(timedelta(seconds=seconds)) == expected + + +@pytest.mark.parametrize( + "seconds,expected", + [ + # Rounding carries into a higher unit, so the ladder re-picks rather + # than rendering "6 d 24 h". + (6 * DAY + 23 * HOUR + 40 * MINUTE, "1 w 0 d"), + (HOUR + 59 * MINUTE + 45, "2 h 00 m"), + ], +) +def test_adaptive_repicks_when_rounding_carries(seconds, expected): + assert _presentation("adaptive").format(timedelta(seconds=seconds)) == expected + + +def test_unregistered_profile_id_raises(): + with pytest.raises(ValueError, match="two_hours"): + duration_format_profile("two_hours") + + +def test_format_decimal_hours_is_preference_independent(): + """Session.__str__ and other request-less callers need one fixed + rendering, not the viewer's profile.""" + assert format_decimal_hours(timedelta(seconds=HOUR + 12 * MINUTE)) == "1.2" + assert format_decimal_hours(None) == "0.0" From de483163b10a2438b528b56c8a1b30f7763bc45e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:17:23 +0200 Subject: [PATCH 02/15] feat: durations group and punctuate numbers by locale force_grouping is passed per call because USE_THOUSAND_SEPARATOR is unset project-wide, and turning it on globally would retroactively regroup every price on the site. The locale override is scoped to the number call so the formatting locale never becomes the application's language. Co-Authored-By: Claude Opus 5 --- common/duration_presentation.py | 17 +++++++++- tests/test_duration_presentation.py | 51 +++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 3 deletions(-) diff --git a/common/duration_presentation.py b/common/duration_presentation.py index 3313438b..2c181c1b 100644 --- a/common/duration_presentation.py +++ b/common/duration_presentation.py @@ -21,6 +21,8 @@ from typing import Protocol from django.http import HttpRequest +from django.utils.formats import number_format +from django.utils.translation import override from timetracker.settings_resolver import resolve_str_for_user @@ -192,8 +194,21 @@ class DurationPresentation: profile: DurationProfile locale: str + def _numbers(self, value: float, decimals: int) -> str: + """Grouping and the decimal separator come from the formatting locale. + + ``force_grouping`` is explicit because ``USE_THOUSAND_SEPARATOR`` is + unset project-wide; turning it on globally would retroactively regroup + every price on the site. ``override`` is scoped to this call for the + same reason ``day_periods_for_locale`` scopes its own — the formatting + locale is deliberately never activated request-wide, so it cannot + change application copy. + """ + with override(self.locale): + return number_format(value, decimals, force_grouping=True) + def format(self, duration: timedelta | float | None) -> str: - return self.profile.render(_total_seconds(duration), plain_number) + return self.profile.render(_total_seconds(duration), self._numbers) _REQUEST_CACHE_ATTRIBUTE = "_duration_presentation" diff --git a/tests/test_duration_presentation.py b/tests/test_duration_presentation.py index 14f266ec..0df4320a 100644 --- a/tests/test_duration_presentation.py +++ b/tests/test_duration_presentation.py @@ -32,8 +32,8 @@ (26 * HOUR, "26.0 h", "26 h 00 m", "26 hours", "1 d 02 h"), (83 * HOUR + 12 * MINUTE, "83.2 h", "83 h 12 m", "83 hours", "3 d 11 h"), (200 * HOUR, "200.0 h", "200 h 00 m", "200 hours", "1 w 1 d"), - (1234 * HOUR, "1234.0 h", "1234 h 00 m", "1234 hours", "7 w 2 d"), - (9000 * HOUR, "9000.0 h", "9000 h 00 m", "9000 hours", "1 y 2 w"), + (1234 * HOUR, "1,234.0 h", "1,234 h 00 m", "1,234 hours", "7 w 2 d"), + (9000 * HOUR, "9,000.0 h", "9,000 h 00 m", "9,000 hours", "1 y 2 w"), ] PROFILE_IDS = ("decimal_hours", "hours_minutes", "whole_hours", "adaptive") @@ -108,3 +108,50 @@ def test_format_decimal_hours_is_preference_independent(): rendering, not the viewer's profile.""" assert format_decimal_hours(timedelta(seconds=HOUR + 12 * MINUTE)) == "1.2" assert format_decimal_hours(None) == "0.0" + + +# The cs thousand separator is U+00A0, not an ASCII space. +CZECH_GROUP = "\xa0" + + +@pytest.mark.parametrize( + "locale,expected", + [("en-us", "1,234 hours"), ("cs", f"1{CZECH_GROUP}234 hours")], +) +def test_grouping_follows_the_locale(locale, expected): + presentation = DurationPresentation( + profile=duration_format_profile("whole_hours"), locale=locale + ) + + assert presentation.format(timedelta(seconds=1234 * HOUR)) == expected + + +@pytest.mark.parametrize("locale,expected", [("en-us", "1.2 h"), ("cs", "1,2 h")]) +def test_decimal_separator_follows_the_locale(locale, expected): + presentation = DurationPresentation( + profile=duration_format_profile("decimal_hours"), locale=locale + ) + + assert presentation.format(timedelta(seconds=HOUR + 12 * MINUTE)) == expected + + +@pytest.mark.parametrize("locale", ["en-us", "cs"]) +def test_small_values_are_not_grouped(locale): + presentation = DurationPresentation( + profile=duration_format_profile("whole_hours"), locale=locale + ) + + assert presentation.format(timedelta(seconds=26 * HOUR)) == "26 hours" + + +def test_formatting_does_not_leak_the_active_translation(): + """The formatting locale must never become the application's language — + common/middleware.py keeps them apart deliberately.""" + from django.utils.translation import get_language + + before = get_language() + DurationPresentation( + profile=duration_format_profile("whole_hours"), locale="cs" + ).format(timedelta(seconds=1234 * HOUR)) + + assert get_language() == before From cadcda6c9a20d415c98d7918f01ee96cee44c4c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:18:47 +0200 Subject: [PATCH 03/15] feat: deduplicated duration alternates and a spoken form Dedup compares rendered strings rather than profile identity: adaptive and hours_minutes agree below a day but diverge at the carry boundary, so 23 h 59 m 45 s legitimately has three distinct alternates. The spoken form is profile-independent and uses hours and minutes only. Screen readers read '1.2 h' as 'one point two h', and no listener is helped by '375 days'. Co-Authored-By: Claude Opus 5 --- common/duration_presentation.py | 42 ++++++++++++++ tests/test_duration_presentation.py | 85 +++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/common/duration_presentation.py b/common/duration_presentation.py index 2c181c1b..1526ea49 100644 --- a/common/duration_presentation.py +++ b/common/duration_presentation.py @@ -210,6 +210,48 @@ def _numbers(self, value: float, decimals: int) -> str: def format(self, duration: timedelta | float | None) -> str: return self.profile.render(_total_seconds(duration), self._numbers) + def alternates( + self, duration: timedelta | float | None + ) -> tuple[tuple[ProfileLabel, str], ...]: + """The same value under the other profiles, in registry order. + + Deduplicated on the *rendered string*, never on profile identity: + hours_minutes and adaptive agree below a day but diverge at the carry + boundary, so "these two profiles are equivalent" is not a rule that + holds. + """ + seconds = _total_seconds(duration) + seen = {self.profile.render(seconds, self._numbers)} + rows: list[tuple[ProfileLabel, str]] = [] + for profile in DURATION_FORMAT_PROFILES.values(): + if profile.id == self.profile.id: + continue + rendered = profile.render(seconds, self._numbers) + if rendered in seen: + continue + seen.add(rendered) + rows.append((profile.label, rendered)) + return tuple(rows) + + def spoken( + self, duration: timedelta | float | None, *, manual: bool = False + ) -> str: + """The value in full words, independent of the visible profile. + + Screen readers mangle abbreviations — "1.2 h" is read "one point two + h", and "3 d 11 h" is worse. Hours and minutes only: no reader is + helped by "375 days". Numbers are ungrouped so no separator is voiced. + """ + total_minutes = _round_half_away(_total_seconds(duration) / MINUTE_SECONDS) + hours, minutes = divmod(total_minutes, 60) + parts = [ + f"{count} {unit}{'' if count == 1 else 's'}" + for count, unit in ((hours, "hour"), (minutes, "minute")) + if count + ] + spoken = " ".join(parts) if parts else "0 hours" + return f"{spoken}, manual" if manual else spoken + _REQUEST_CACHE_ATTRIBUTE = "_duration_presentation" diff --git a/tests/test_duration_presentation.py b/tests/test_duration_presentation.py index 0df4320a..5a02c04e 100644 --- a/tests/test_duration_presentation.py +++ b/tests/test_duration_presentation.py @@ -155,3 +155,88 @@ def test_formatting_does_not_leak_the_active_translation(): ).format(timedelta(seconds=1234 * HOUR)) assert get_language() == before + + +def _alternates(profile_id: str, seconds: int): + return _presentation(profile_id).alternates(timedelta(seconds=seconds)) + + +def test_alternates_exclude_the_visible_rendering(): + visible = _presentation("decimal_hours").format( + timedelta(seconds=HOUR + 12 * MINUTE) + ) + + renderings = [ + text for _label, text in _alternates("decimal_hours", HOUR + 12 * MINUTE) + ] + + assert visible not in renderings + + +def test_alternates_drop_duplicates_below_24h(): + """hours_minutes and adaptive agree below a day, so the popover shows the + value once, not twice.""" + renderings = [ + text for _label, text in _alternates("decimal_hours", HOUR + 12 * MINUTE) + ] + + assert renderings == ["1 h 12 m", "1 hour"] + + +def test_alternates_keep_all_three_above_24h(): + renderings = [ + text for _label, text in _alternates("decimal_hours", 83 * HOUR + 12 * MINUTE) + ] + + assert renderings == ["83 h 12 m", "83 hours", "3 d 11 h"] + + +def test_adaptive_and_hours_minutes_diverge_at_the_carry_boundary(): + """23 h 59 m 45 s rounds to 24 h 00 m under hours_minutes but re-picks to + 1 d 00 h under adaptive — which is why dedup compares rendered strings and + never profile identity.""" + renderings = [ + text + for _label, text in _alternates("decimal_hours", 23 * HOUR + 59 * MINUTE + 45) + ] + + assert renderings == ["24 h 00 m", "24 hours", "1 d 00 h"] + + +def test_alternates_are_labelled_with_their_profile(): + labels = [label for label, _text in _alternates("decimal_hours", 83 * HOUR)] + + assert labels == ["Hours and minutes", "Whole hours", "Adaptive units"] + + +@pytest.mark.parametrize( + "seconds,expected", + [ + (0, "0 hours"), + (45, "1 minute"), + (45 * MINUTE, "45 minutes"), + (HOUR + 12 * MINUTE, "1 hour 12 minutes"), + (2 * HOUR, "2 hours"), + (9000 * HOUR, "9000 hours"), + # Never days or weeks: "375 days" helps nobody. + (9 * DAY, "216 hours"), + ], +) +def test_spoken_uses_words_and_pluralizes(seconds, expected): + assert _presentation("adaptive").spoken(timedelta(seconds=seconds)) == expected + + +def test_spoken_is_the_same_whatever_the_visible_profile(): + value = timedelta(seconds=HOUR + 12 * MINUTE) + + spoken = {_presentation(profile_id).spoken(value) for profile_id in PROFILE_IDS} + + assert spoken == {"1 hour 12 minutes"} + + +def test_spoken_marks_a_manual_session(): + spoken = _presentation("decimal_hours").spoken( + timedelta(seconds=HOUR + 12 * MINUTE), manual=True + ) + + assert spoken == "1 hour 12 minutes, manual" From 9c7aa12efee27b184cc44da7b9dac2c87c2d0708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:21:06 +0200 Subject: [PATCH 04/15] feat: register the duration format preference DURATION_FORMAT is a user-scoped SELECT resolving through the same chain as DATETIME_FORMAT, defaulting to decimal_hours so an untouched install renders exactly as it did. No migration: unmapped keys fall through UserPreferences.extra_preferences. Co-Authored-By: Claude Opus 5 --- e2e/test_admin_settings_page_e2e.py | 1 + tests/test_admin_settings_page.py | 5 ++- tests/test_duration_setting.py | 59 +++++++++++++++++++++++++++++ tests/test_settings_registry.py | 1 + timetracker/settings_registry.py | 34 +++++++++++++++++ 5 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 tests/test_duration_setting.py diff --git a/e2e/test_admin_settings_page_e2e.py b/e2e/test_admin_settings_page_e2e.py index 8eb0a987..6542c79e 100644 --- a/e2e/test_admin_settings_page_e2e.py +++ b/e2e/test_admin_settings_page_e2e.py @@ -19,6 +19,7 @@ "DISPLAY_TIME_ZONE", "DATE_FORMAT_LOCALE", "DATETIME_FORMAT", + "DURATION_FORMAT", ) diff --git a/tests/test_admin_settings_page.py b/tests/test_admin_settings_page.py index 4355f1ea..6481b3cc 100644 --- a/tests/test_admin_settings_page.py +++ b/tests/test_admin_settings_page.py @@ -32,6 +32,7 @@ "SESSION_TIME_ZONE_DISPLAY", "DATE_FORMAT_LOCALE", "DATETIME_FORMAT", + "DURATION_FORMAT", ) @@ -323,8 +324,8 @@ def test_admin_page_lists_device_rows_and_select_options( # One per USER-scope SELECT setting: default_landing_page, default_page_size, # theme, display_time_zone, session_time_zone_display, date_format_locale, - # datetime_format (default_currency/default_device are not SELECT widgets). - assert html.count(">Use configured default") == 8 + # datetime_format, duration_format (default_currency is not a SELECT widget). + assert html.count(">Use configured default") == 9 assert html.index( f'' ) < html.index(">Steam Deck (Handheld)") diff --git a/tests/test_duration_setting.py b/tests/test_duration_setting.py new file mode 100644 index 00000000..0f727b4b --- /dev/null +++ b/tests/test_duration_setting.py @@ -0,0 +1,59 @@ +"""DURATION_FORMAT resolves through the same chain every user preference does.""" + +import pytest +from django.core.exceptions import ValidationError +from django.test import RequestFactory + +from common.duration_presentation import duration_presentation_for_request +from timetracker.settings_commands import change_site_setting, change_user_setting +from timetracker.settings_resolver import resolve_str_for_user + + +@pytest.fixture +def user(django_user_model, db): + return django_user_model.objects.create_user(username="u", password="p") + + +def test_default_is_decimal_hours(user): + assert resolve_str_for_user(user, "DURATION_FORMAT") == "decimal_hours" + + +def test_personal_value_overrides_the_site_default(user): + change_site_setting("DURATION_FORMAT", "whole_hours") + change_user_setting(user, "DURATION_FORMAT", "adaptive") + + assert resolve_str_for_user(user, "DURATION_FORMAT") == "adaptive" + + +def test_clearing_the_personal_value_restores_the_site_default(user): + change_site_setting("DURATION_FORMAT", "whole_hours") + change_user_setting(user, "DURATION_FORMAT", "adaptive") + + change_user_setting(user, "DURATION_FORMAT", None) + + assert resolve_str_for_user(user, "DURATION_FORMAT") == "whole_hours" + + +@pytest.mark.parametrize("value", ["two_hours", "", "ADAPTIVE!"]) +def test_unregistered_profile_is_rejected(user, value): + with pytest.raises(ValidationError): + change_user_setting(user, "DURATION_FORMAT", value) + + +def test_presentation_is_cached_on_the_request(user): + request = RequestFactory().get("/tracker/session/list") + request.user = user + + first = duration_presentation_for_request(request) + second = duration_presentation_for_request(request) + + assert first is second + assert first.profile.id == "decimal_hours" + + +def test_presentation_follows_the_personal_preference(user): + change_user_setting(user, "DURATION_FORMAT", "adaptive") + request = RequestFactory().get("/tracker/session/list") + request.user = user + + assert duration_presentation_for_request(request).profile.id == "adaptive" diff --git a/tests/test_settings_registry.py b/tests/test_settings_registry.py index f12ee727..36a8afd8 100644 --- a/tests/test_settings_registry.py +++ b/tests/test_settings_registry.py @@ -27,6 +27,7 @@ "DISPLAY_TIME_ZONE", "DATE_FORMAT_LOCALE", "DATETIME_FORMAT", + "DURATION_FORMAT", "SESSION_TIME_ZONE_DISPLAY", } diff --git a/timetracker/settings_registry.py b/timetracker/settings_registry.py index b6120f06..9bcfe5d2 100644 --- a/timetracker/settings_registry.py +++ b/timetracker/settings_registry.py @@ -71,6 +71,15 @@ _DATETIME_FORMAT_VALUES: Final[frozenset[str]] = frozenset( value for value, _label in DATETIME_FORMAT_CHOICES ) +DURATION_FORMAT_CHOICES: Final[tuple[tuple[str, str], ...]] = ( + ("decimal_hours", "Decimal hours (1.2 h)"), + ("hours_minutes", "Hours and minutes (1 h 12 m)"), + ("whole_hours", "Whole hours (1 hour)"), + ("adaptive", "Adaptive units (3 d 11 h)"), +) +_DURATION_FORMAT_VALUES: Final[frozenset[str]] = frozenset( + value for value, _label in DURATION_FORMAT_CHOICES +) DISPLAY_TIME_ZONE_CHOICES: Final[tuple[tuple[str, str], ...]] = tuple( (time_zone, time_zone) for time_zone in sorted(available_timezones()) ) @@ -263,6 +272,16 @@ def _validate_datetime_format(value: object) -> str: return normalized +def _validate_duration_format(value: object) -> str: + normalized = value.strip().lower() if isinstance(value, str) else value + if not isinstance(normalized, str) or normalized not in _DURATION_FORMAT_VALUES: + choices = ", ".join(sorted(_DURATION_FORMAT_VALUES)) + raise ValidationError( + f"Duration format must be one of {choices} (got {value!r})." + ) + return normalized + + def _validate_session_time_zone_display(value: object) -> str: normalized = value.strip().lower() if isinstance(value, str) else value if ( @@ -408,6 +427,21 @@ def _build_registry() -> dict[SettingKey, SettingDefinition]: choices=DATETIME_FORMAT_CHOICES, reload_after_save=True, ), + SettingDefinition( + "DURATION_FORMAT", + scope=SettingScope.USER, + apply_timing=ApplyTiming.LIVE, + label="Duration format", + help_text=( + "How elapsed playtime is displayed. Every duration also offers " + "the other formats on hover." + ), + default_factory=lambda: "decimal_hours", + validator=_validate_duration_format, + widget=SettingWidget.SELECT, + choices=DURATION_FORMAT_CHOICES, + reload_after_save=True, + ), SettingDefinition( "TZ", scope=SettingScope.INFRA, From bc297e6d936bcd84343fbc18cbc6b2be158853de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:22:02 +0200 Subject: [PATCH 05/15] feat: popovers can opt out of aria-describedby Co-Authored-By: Claude Opus 5 --- common/components/primitives.py | 18 +++++++++-------- tests/test_components.py | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/common/components/primitives.py b/common/components/primitives.py index a14f1aaa..86a9cea1 100644 --- a/common/components/primitives.py +++ b/common/components/primitives.py @@ -325,6 +325,7 @@ def _popover_html( trigger_disabled: bool = False, preface: Node | str = "", selectable_text: bool = False, + describedby: bool = True, ) -> Node: """Generate popover HTML. Single source of truth for popover structure. @@ -391,19 +392,18 @@ def _popover_html( Button(control_attributes)[*trigger_children] ] else: - control_attributes.append(("aria-describedby", id)) + if describedby: + control_attributes.append(("aria-describedby", id)) if trigger_label: control_attributes.append(("aria-label", trigger_label)) control_attributes.append(("data-pop-over-trigger", "")) trigger = Button(control_attributes)[*trigger_children] else: - trigger = Span( - [ - ("data-pop-over-trigger", ""), - ("aria-describedby", id), - ("class", wrapped_classes), - ] - )[*trigger_children] + span_attributes: list[HTMLAttribute] = [("data-pop-over-trigger", "")] + if describedby: + span_attributes.append(("aria-describedby", id)) + span_attributes.append(("class", wrapped_classes)) + trigger = Span(span_attributes)[*trigger_children] # No positioning class — the element sets `position: fixed` + coords on show # and clears them on hide; the `hidden` attribute owns the closed state. @@ -443,6 +443,7 @@ def Popover( trigger_disabled: bool = False, preface: Node | str = "", selectable_text: bool = False, + describedby: bool = True, ) -> Node: children = as_children(children) if not wrapped_content and not children: @@ -462,6 +463,7 @@ def Popover( trigger_disabled=trigger_disabled, preface=preface, selectable_text=selectable_text, + describedby=describedby, ) diff --git a/tests/test_components.py b/tests/test_components.py index efa9cc47..3c9d9405 100644 --- a/tests/test_components.py +++ b/tests/test_components.py @@ -2755,3 +2755,39 @@ def test_confirm_page_renders_details_outside_the_message_paragraph(): if __name__ == "__main__": unittest.main() + + +class PopoverDescribedbyTest(unittest.TestCase): + """A popover whose panel only restates the trigger can drop the + description, so a screen reader does not read the value twice.""" + + def test_popover_omits_describedby_when_disabled(self): + html = str( + components.Popover( + popover_content="1 h 12 m", + wrapped_content="1.2 h", + id="duration-1", + describedby=False, + ) + ) + self.assertNotIn("aria-describedby", html) + + def test_panel_keeps_its_id_and_tooltip_role_when_describedby_is_off(self): + html = str( + components.Popover( + popover_content="1 h 12 m", + wrapped_content="1.2 h", + id="duration-2", + describedby=False, + ) + ) + self.assertIn('id="duration-2"', html) + self.assertIn('role="tooltip"', html) + + def test_describedby_is_present_by_default(self): + html = str( + components.Popover( + popover_content="original", wrapped_content="converted", id="price-1" + ) + ) + self.assertIn('aria-describedby="price-1"', html) From 0b6f2b8ed73fdee01b1eb117ffbf8dbe473f4555 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:23:05 +0200 Subject: [PATCH 06/15] feat: Duration renders a value with its alternate formats MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The id_scope parameter is required: Popover derives its DOM id by hashing its own content, so two rows showing the same duration would collide, and Game.playtime defaults to zero — which makes that the common case on a game list rather than an edge case. Co-Authored-By: Claude Opus 5 --- common/components/__init__.py | 2 + common/components/domain.py | 52 +++++++++++++++++++ tests/test_duration_component.py | 86 ++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 tests/test_duration_component.py diff --git a/common/components/__init__.py b/common/components/__init__.py index eaa939a4..cb8e03dc 100644 --- a/common/components/__init__.py +++ b/common/components/__init__.py @@ -64,6 +64,7 @@ ) from common.components.domain import ( BrowserTimeZoneInput, + Duration, GameLink, GameStatus, GameStatusSelector, @@ -273,6 +274,7 @@ "DropdownMenuPanel", "DropdownPostItem", "DropdownSubmenu", + "Duration", "Element", "ExternalScript", "FilterBuilder", diff --git a/common/components/domain.py b/common/components/domain.py index 983230e2..ba1ddc1d 100644 --- a/common/components/domain.py +++ b/common/components/domain.py @@ -10,6 +10,7 @@ ICON_BUTTON_SIZE_CLASS, NAME_MAX_WIDTH_CLASS, A, + Div, Icon, Input, Li, @@ -23,6 +24,7 @@ from games.models import Game, Purchase, Session if TYPE_CHECKING: + from common.duration_presentation import DurationPresentation from common.returns import OriginUrl @@ -338,6 +340,56 @@ def SessionDeviceSelector(session, session_devices, csrf_token: str) -> Node: ) +def Duration( + duration, + presentation: DurationPresentation, + *, + id_scope: str, + manual: bool = False, +) -> Node: + """One elapsed duration, with the same value under the other profiles on hover. + + ``id_scope`` is required and must be unique on the page. ``Popover`` derives + its DOM id by hashing its own content, so two rows showing the same duration + would collide — and ``Game.playtime`` defaults to zero, which makes that the + common case on a game list rather than an edge case. + + The visible text is ``aria-hidden`` and a sibling ``sr-only`` span carries + the value in words: screen readers read "1.2 h" as "one point two h". For + the same reason the panel drops ``aria-describedby`` — it restates what the + ``sr-only`` text already said. + + ``manual`` appends the "*" mark that flags a hand-entered session. It sits + inside the trigger with the value and is spoken as ", manual"; it qualifies + the value, not its formatting, so it never appears among the alternates. + """ + from common.components.primitives import Popover + + visible = presentation.format(duration) + alternates = presentation.alternates(duration) + rows = [ + Div(class_="flex justify-between gap-4")[ + Span(class_="text-neutral-secondary")[label], + Span(class_="tabular-nums")[rendering], + ] + for label, rendering in alternates + ] + + trigger = Fragment( + Span(aria_hidden="true")[f"{visible}*" if manual else visible], + Span(class_="sr-only")[presentation.spoken(duration, manual=manual)], + ) + + return Popover( + popover_content=Div(class_="flex flex-col gap-1 text-left")[*rows], + children=[trigger], + wrapped_classes="tabular-nums underline decoration-dotted", + id=f"duration-{id_scope}", + describedby=False, + selectable_text=True, + ) + + BROWSER_TIME_ZONE_FIELD = "browser_time_zone" diff --git a/tests/test_duration_component.py b/tests/test_duration_component.py new file mode 100644 index 00000000..955dc217 --- /dev/null +++ b/tests/test_duration_component.py @@ -0,0 +1,86 @@ +"""Duration(): the visible rendering, its alternates, and its spoken form.""" + +from datetime import timedelta + +import pytest + +from common.components import Duration +from common.components.core import assert_unique_element_ids +from common.duration_presentation import ( + DurationPresentation, + duration_format_profile, +) + +MINUTE = 60 +HOUR = 60 * MINUTE + + +def _presentation(profile_id: str = "decimal_hours") -> DurationPresentation: + return DurationPresentation( + profile=duration_format_profile(profile_id), locale="en-us" + ) + + +def _render(seconds: int, *, id_scope: str = "session-1-duration", **kwargs) -> str: + return str( + Duration( + timedelta(seconds=seconds), + _presentation(), + id_scope=id_scope, + **kwargs, + ) + ) + + +def test_visible_value_is_hidden_from_assistive_technology(): + html = _render(HOUR + 12 * MINUTE) + + assert 'aria-hidden="true"' in html + assert "1.2 h" in html + + +def test_spoken_text_is_rendered_sr_only(): + html = _render(HOUR + 12 * MINUTE) + + assert 'class="sr-only"' in html + assert "1 hour 12 minutes" in html + + +def test_manual_mark_follows_the_value_and_is_spoken(): + html = _render(HOUR + 12 * MINUTE, manual=True) + + assert "1.2 h*" in html + assert "1 hour 12 minutes, manual" in html + + +def test_alternates_render_as_label_value_rows(): + html = _render(HOUR + 12 * MINUTE) + + assert "Hours and minutes" in html + assert "1 h 12 m" in html + assert "Whole hours" in html + assert "1 hour" in html + + +def test_describedby_is_absent(): + """The sr-only text already carries the value; describing the trigger with + the panel would read the same number three times per row.""" + assert "aria-describedby" not in _render(HOUR + 12 * MINUTE) + + +def test_two_equal_durations_get_distinct_ids(): + """Popover derives an id by hashing its content, so equal durations on one + page would collide without a caller-supplied scope.""" + from common.components import Fragment + + document = Fragment( + Duration(timedelta(0), _presentation(), id_scope="game-1-playtime"), + Duration(timedelta(0), _presentation(), id_scope="game-2-playtime"), + ) + + assert_unique_element_ids(str(document)) + + +def test_id_scope_is_required(): + with pytest.raises(TypeError): + Duration(timedelta(0), _presentation()) # type: ignore[call-arg] From c14282e7443ec789236486dc8bf3139b9386f62f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Kucharczyk?= Date: Wed, 29 Jul 2026 13:31:37 +0200 Subject: [PATCH 07/15] feat: sessions, games, and the navbar use the duration formatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Duration splits into DurationText (the value plus its spoken form) and DurationAlternates (the other profiles), so a surface that already owns a popover can show both without nesting one popover inside another. The game-detail stats row does exactly that. NavbarPlaytime is a node tree rather than an HTML f-string, which is what lets a duration node's Media survive there at all. The navbar totals still render as text: each already links to its filtered session list, and a popover trigger is a