diff --git a/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md b/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md index ef9219d5..dd04ba8a 100644 --- a/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md +++ b/.github/PULL_REQUEST_TEMPLATE/development-to-staging.md @@ -6,7 +6,7 @@ Leave a category empty (or delete its header) if nothing applies. --> -## User-visible improvements (UVIs) +## Summary ### Features - @@ -19,9 +19,8 @@ --- +## Contributors + + ## Technical notes - -## Test plan -- [ ] CI passes -- [ ] Smoke test on staging after deploy diff --git a/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md b/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md index c27096c0..e556e256 100644 --- a/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md +++ b/.github/PULL_REQUEST_TEMPLATE/staging-to-main.md @@ -19,9 +19,8 @@ --- +## Contributors + + ## Technical notes - -## Test plan -- [ ] CI passes -- [ ] Verified on staging diff --git a/Makefile b/Makefile index 47affdfb..28341cf8 100755 --- a/Makefile +++ b/Makefile @@ -26,6 +26,8 @@ ds: t: docker compose exec web python manage.py test $(t) --keepdb --buffer +tnk: + docker compose exec web python manage.py test $(t) --buffer vt: cd ./frontend diff --git a/api/tests.py b/api/tests.py index 6a4d5fe8..8c0bac4a 100644 --- a/api/tests.py +++ b/api/tests.py @@ -32,7 +32,7 @@ def player_for(user) -> Player: class TestMeViewSet(APITestCase): def setUp(self): - self.character = Character.objects.create(given_name="Hero", can_link=True) + self.character = Character.objects.create(given_name="Hero") self.user = create_test_user( email="duncan@example.com", password="pass12345", @@ -194,32 +194,78 @@ def test_complete_onboarding_sets_flag(self): self.assertTrue(player_for(self.user).onboarding_completed) self.assertEqual(res.data, {"onboarding_completed": True}) - def test_today_points_null_when_no_active_link(self): + def test_daily_goals_null_when_no_active_link(self): self.authenticate() player = player_for(self.user) self.assertIsNone(player.active_link) - res = self.client.get(reverse("me-today-points")) + res = self.client.get(reverse("me-daily-goals")) self.assertEqual(res.status_code, status.HTTP_200_OK) - self.assertIsNone(res.data["points_today"]) + self.assertIsNone(res.data["goals"]) - def test_today_points_reflects_todays_completed_activities(self): + def test_daily_goals_all_false_when_linked_but_no_activity_or_login(self): self.authenticate() player = player_for(self.user) PlayerCharacterLink.objects.create(player=player, character=self.character) + res = self.client.get(reverse("me-daily-goals")) + + self.assertEqual(res.status_code, status.HTTP_200_OK) + goals = res.data["goals"] + self.assertFalse(goals["logged_in_today"]) + self.assertFalse(goals["completed_activity_today"]) + self.assertEqual(goals["activity_minutes_today"], 0) + self.assertFalse(goals["minutes_goal_met"]) + self.assertFalse(goals["all_goals_met"]) + self.assertFalse(goals["bonus_awarded_today"]) + self.assertEqual(goals["bonus_ap"], 0) + + def test_daily_goals_reflects_todays_completed_activities(self): + self.authenticate() + player = player_for(self.user) + PlayerCharacterLink.objects.create(player=player, character=self.character) + + PlayerActivity.objects.create( + player=player, + is_complete=True, + duration=1200, # 20 minutes + completed_at=datetime.now(timezone.utc), + ) + + res = self.client.get(reverse("me-daily-goals")) + + self.assertEqual(res.status_code, status.HTTP_200_OK) + goals = res.data["goals"] + self.assertTrue(goals["completed_activity_today"]) + self.assertEqual(goals["activity_minutes_today"], 20) + self.assertTrue(goals["minutes_goal_met"]) + + def test_daily_goals_all_met_awards_bonus_once(self): + from users.models import UserLogin + + self.authenticate() + player = player_for(self.user) + PlayerCharacterLink.objects.create(player=player, character=self.character) + UserLogin.objects.create(user=self.user) + PlayerActivity.objects.create( player=player, is_complete=True, - duration=1200, # 20 minutes -> 2 points + duration=1200, completed_at=datetime.now(timezone.utc), ) - res = self.client.get(reverse("me-today-points")) + res = self.client.get(reverse("me-daily-goals")) self.assertEqual(res.status_code, status.HTTP_200_OK) - self.assertEqual(res.data["points_today"], 2) + goals = res.data["goals"] + self.assertTrue(goals["all_goals_met"]) + # Reads never award the bonus themselves - only activity completion + # does (via check_and_award_daily_goals), so a plain GET here + # shouldn't have paid it out. + self.assertFalse(goals["bonus_awarded_today"]) + self.assertEqual(goals["bonus_ap"], 0) class CustomTokenObtainPairViewTests(APITestCase): diff --git a/api/urls.py b/api/urls.py index 947d9a01..3b94c728 100644 --- a/api/urls.py +++ b/api/urls.py @@ -43,6 +43,7 @@ from locations.views import ( PopulationCentreMapView, + InitialMapCentreView, MapCharacterDetailView, MapViewportView, MapWorldBoundsView, @@ -135,6 +136,11 @@ def to_url(self, value): PopulationCentreMapView.as_view(), name="populationcentre-map", ), + path( + "map/initial-centre/", + InitialMapCentreView.as_view(), + name="map-initial-centre", + ), path( "map/viewport/", MapViewportView.as_view(), diff --git a/api/views.py b/api/views.py index 65505a45..4d67fa11 100644 --- a/api/views.py +++ b/api/views.py @@ -363,24 +363,61 @@ def character(self, request): @extend_schema( responses=inline_serializer( - name="TodayPointsResponse", + name="DailyGoalsResponse", fields={ - "points_today": drf_serializers.IntegerField(allow_null=True), + "goals": inline_serializer( + name="DailyGoalsStateResponse", + fields={ + "logged_in_today": drf_serializers.BooleanField(), + "completed_activity_today": drf_serializers.BooleanField(), + "activity_minutes_today": drf_serializers.IntegerField(), + "minutes_goal_threshold": drf_serializers.IntegerField(), + "minutes_goal_met": drf_serializers.BooleanField(), + "all_goals_met": drf_serializers.BooleanField(), + "bonus_awarded_today": drf_serializers.BooleanField(), + "bonus_ap": drf_serializers.IntegerField(), + }, + allow_null=True, + ), }, ) ) @action(detail=False, methods=["get"]) - def today_points(self, request): + def daily_goals(self, request): """ - Personal "points earned today" for the map view's badge (issue #673). - `points_today` is null - not zero - when the player has no active + Daily goals for the map view's badge (issue #751, replacing the old + `today_points`/`points_today` mechanic from issue #673). `goals` is + null - not a set of all-false goals - when the player has no active PlayerCharacterLink, so the frontend can tell "no link" apart from - "linked but nothing earned yet today" and hide the badge entirely. + "linked but no goals cleared yet today" and hide the badge entirely. """ + from progression.daily_goals import ( + MINUTES_GOAL_THRESHOLD, + get_daily_goals_state, + ) + player = request.user.player link = player.active_link - return Response({"points_today": link.points_today if link else None}) + if not link: + return Response({"goals": None}) + + state = get_daily_goals_state(player) + + return Response( + { + "goals": { + "logged_in_today": state.logged_in_today, + "completed_activity_today": state.completed_activity_today, + "activity_minutes_today": state.activity_minutes_today, + "minutes_goal_threshold": MINUTES_GOAL_THRESHOLD, + "minutes_goal_met": state.minutes_goal_met, + "all_goals_met": state.all_goals_met, + "bonus_awarded_today": state.bonus_awarded_today, + "bonus_ap": state.bonus_ap, + } + } + ) @extend_schema( responses=inline_serializer( diff --git a/character/admin.py b/character/admin.py index d9c5a8bf..f8015d5b 100644 --- a/character/admin.py +++ b/character/admin.py @@ -60,6 +60,27 @@ def get_other_members(self, obj): return ", ".join(str(c) for c in others) +class CanLinkListFilter(admin.SimpleListFilter): + """ + can_link is a derived property, not a DB column, so it can't be listed + in list_filter directly - filter via Character.objects.linkable() + (the queryset-level equivalent) instead. + """ + + title = "can link" + parameter_name = "can_link" + + def lookups(self, request, model_admin): + return (("yes", "Yes"), ("no", "No")) + + def queryset(self, request, queryset): + if self.value() == "yes": + return queryset.filter(pk__in=Character.objects.linkable()) + if self.value() == "no": + return queryset.exclude(pk__in=Character.objects.linkable()) + return queryset + + @admin.action(description="Mark selected characters as NPCs and unlink from players") def mark_as_npc(modeladmin, request, queryset): for character in queryset: @@ -73,17 +94,6 @@ def mark_as_npc(modeladmin, request, queryset): ) -@admin.action(description="Mark selected characters as available to link") -def mark_as_canlink(modeladmin, request, queryset): - for character in queryset: - character.can_link = True - character.save(update_fields=["can_link"]) - - messages.success( - request, f"{queryset.count()} character(s) marked as available to link." - ) - - @admin.register(Character) class CharacterAdmin(admin.ModelAdmin): fieldsets = ( @@ -93,6 +103,7 @@ class CharacterAdmin(admin.ModelAdmin): "fields": ( "given_name", "can_link", + "is_reserved", "sex", ) }, @@ -145,7 +156,8 @@ class CharacterAdmin(admin.ModelAdmin): "birth_date", ] list_filter = [ - "can_link", + CanLinkListFilter, + "is_reserved", "birth_date", "death_date", "sex", @@ -156,6 +168,7 @@ class CharacterAdmin(admin.ModelAdmin): "links__player__name", ] readonly_fields = [ + "can_link", "get_player", "get_age", "created_at", @@ -168,7 +181,7 @@ class CharacterAdmin(admin.ModelAdmin): CharacterRelationshipMembershipInline, CharacterCurrencyInline, ] - actions = [mark_as_npc, mark_as_canlink] + actions = [mark_as_npc] @admin.display(description="Player") def get_player(self, obj): diff --git a/character/filters.py b/character/filters.py index c8a3b864..59d7127f 100644 --- a/character/filters.py +++ b/character/filters.py @@ -7,7 +7,7 @@ class CharacterFilter(django_filters.FilterSet): level = django_filters.RangeFilter(field_name="level") xp = django_filters.RangeFilter(field_name="xp") is_npc = django_filters.BooleanFilter(method="filter_is_npc") - can_link = django_filters.BooleanFilter(field_name="can_link") + can_link = django_filters.BooleanFilter(method="filter_can_link") class Meta: model = Character @@ -21,3 +21,15 @@ def filter_is_npc(self, queryset, name, value): else: # is_npc=False means HAS active player link return queryset.filter(links__is_active=True) + + def filter_can_link(self, queryset, name, value): + """ + can_link is derived, not a DB column - defer to + Character.objects.linkable(), the queryset-level equivalent of + Character.can_link, instead of duplicating its logic here. + """ + linkable = Character.objects.linkable() + if value: + return queryset.filter(pk__in=linkable) + else: + return queryset.exclude(pk__in=linkable) diff --git a/character/migrations/0021_remove_character_can_link_character_is_reserved.py b/character/migrations/0021_remove_character_can_link_character_is_reserved.py new file mode 100644 index 00000000..c74dd925 --- /dev/null +++ b/character/migrations/0021_remove_character_can_link_character_is_reserved.py @@ -0,0 +1,26 @@ +# Generated by Django 5.2.17 on 2026-08-13 16:31 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("character", "0020_remove_character_building"), + ] + + operations = [ + migrations.RemoveField( + model_name="character", + name="can_link", + ), + migrations.AddField( + model_name="character", + name="is_reserved", + field=models.BooleanField( + default=False, + help_text="Manually held back from linking (e.g. reserved for a future storyline), independent of age or link status.", + verbose_name="Reserved", + ), + ), + ] diff --git a/character/models/character.py b/character/models/character.py index fab8ffc6..2bb024db 100644 --- a/character/models/character.py +++ b/character/models/character.py @@ -1,4 +1,5 @@ # from datetime import datetime +from datetime import timedelta from celery import current_app from decimal import Decimal from django.contrib.gis.geos import Point @@ -309,6 +310,33 @@ def get_miscarriage_change(self): ######################################################################## +class CharacterQuerySet(models.QuerySet): + def linkable(self): + """ + Characters currently eligible to be linked to a player - the + queryset-level equivalent of `Character.can_link`, for call sites + that need to filter/exist-check in SQL rather than load instances. + Keep this in sync with `Character.can_link` by hand; there's no + way to share the logic verbatim since one runs in the DB and the + other in Python. + """ + cutoff_date = timezone.now().date() - timedelta( + days=Character.MIN_LINK_AGE_DAYS + ) + return ( + self.filter(is_reserved=False) + .filter( + models.Q(birth_date__isnull=True) + | models.Q(birth_date__lte=cutoff_date) + ) + .exclude(links__is_active=True) + ) + + +class CharacterManager(models.Manager.from_queryset(CharacterQuerySet)): # type: ignore[misc] + pass + + class Character(LevelProgressionMixin, LifeCycleMixin, Movable): class SexChoices(models.TextChoices): MALE = "Male", "Male" @@ -335,11 +363,21 @@ class SexChoices(models.TextChoices): max_length=20, choices=SexChoices.choices, null=True, blank=True ) reputation = models.IntegerField(default=0) - can_link = models.BooleanField(default=False) + is_reserved = models.BooleanField( + default=False, + verbose_name="Reserved", + help_text="Manually held back from linking (e.g. reserved for a future storyline), independent of age or link status.", + ) link_points_multiplier = models.DecimalField( max_digits=5, decimal_places=2, default="1.00" ) + objects = CharacterManager() + + # Minimum age (in days) for a character to be linkable. ~18 years, + # matching spawn-time character generation. + MIN_LINK_AGE_DAYS = int(18 * 365.25) + @property def is_npc(self): """ @@ -347,6 +385,33 @@ def is_npc(self): """ return not self.links.filter(is_active=True).exists() + @property + def is_underage(self): + # An unknown birth_date isn't evidence of being underage - it just + # means age isn't gating linkability for this character. + if self.birth_date is None: + return False + return self.get_age() < self.MIN_LINK_AGE_DAYS + + @property + def can_link(self) -> bool: + """ + Whether this character is currently eligible to be linked to a + player, derived from independent reasons - manual reservation, + age, active link, and (once population centres can gate linking - + see #681) population_centre.characters_can_link - rather than a + flag several call sites could clobber. Keep in sync by hand with + `CharacterQuerySet.linkable`, the SQL-level equivalent used where + a Python loop over instances isn't practical. + """ + if self.is_reserved: + return False + if self.is_underage: + return False + if self.links.filter(is_active=True).exists(): + return False + return True + def __str__(self): return self.name @@ -455,6 +520,15 @@ def total_link_points(self): """ return PlayerCharacterLink.total_link_points(self.links.all()) + def get_productivity(self, now=None): + """ + Live productivity signal - see progression.ap.get_productivity for + what drives it (authored baseline x current active XpModifiers). + """ + from progression import ap + + return ap.get_productivity(self, now=now) + ######################################################################## #### PLAYER CHARACTER LINK MODEL @@ -520,34 +594,6 @@ def link_points(self): ) return int(base_points * multiplier) - @property - def player_time_today(self): - """ - Completed activity time for this link so far today (in minutes) - - a date-filtered variant of player_time, bounded to the current UTC - day instead of the whole link lifetime. - """ - start_of_day = timezone.now().replace(hour=0, minute=0, second=0, microsecond=0) - start = max(start_of_day, self.linked_at) - - qs = self.player.activities.filter(is_complete=True, completed_at__gte=start) - if self.unlinked_at: - qs = qs.filter(completed_at__lte=self.unlinked_at) - - total_seconds = qs.aggregate(total=Sum("duration"))["total"] or 0 - return int(total_seconds // 60) - - @property - def points_today(self): - """ - Points earned today (issue #673's map-view "today" badge) - only the - activity-time component of link_points, date-filtered to today. - link_points' other terms (days_linked * 20, login_points) aren't - "earned today" in the same sense, so they're deliberately left out - here rather than prorated. - """ - return self.player_time_today // 10 - @classmethod def get_character(cls, player: Player) -> Character: return link_services.player_link_get_character(cls, player) diff --git a/character/services/character_services.py b/character/services/character_services.py index c2cd135b..6711fb12 100644 --- a/character/services/character_services.py +++ b/character/services/character_services.py @@ -40,6 +40,4 @@ def character_assign_work(character, building) -> None: def character_has_available(model_cls) -> bool: - return ( - model_cls.objects.filter(can_link=True).exclude(links__is_active=True).exists() - ) + return model_cls.objects.linkable().exists() diff --git a/character/services/link_services.py b/character/services/link_services.py index f0ff0816..3af344ba 100644 --- a/character/services/link_services.py +++ b/character/services/link_services.py @@ -37,8 +37,6 @@ def player_link_unlink(link) -> None: link.unlinked_at = timezone.now() link.is_active = False link.save() - link.character.can_link = True - link.character.save(update_fields=["can_link"]) def player_link_deactivate_active_links(model_cls, player) -> None: @@ -59,6 +57,4 @@ def player_link_assign_character(model_cls, player, character): character=character, is_active=True, ) - character.can_link = False - character.save(update_fields=["can_link"]) return link diff --git a/character/signals.py b/character/signals.py index 020e38e2..ec499903 100644 --- a/character/signals.py +++ b/character/signals.py @@ -1,7 +1,6 @@ # character.signals from datetime import timedelta, datetime, time -from django.db import transaction -from django.db.models.signals import pre_save, post_save, pre_delete, post_delete +from django.db.models.signals import post_save, pre_delete, post_delete from django.dispatch import receiver from django.utils import timezone @@ -41,59 +40,6 @@ def unlink_before_deletion(sender, instance, **kwargs): instance.unlink() -def recompute_character_flags(character_id: int) -> None: - """ - Recompute denormalised flags for a Character based on current links. - A character can_link if they have no active player links. - """ - if not character_id: - return - - # Character can link if they don't have any active player links - has_active_link = PlayerCharacterLink.objects.filter( - character_id=character_id, is_active=True - ).exists() - - can_link = not has_active_link - - Character.objects.filter(id=character_id).update( - can_link=can_link, - ) - - -@receiver(pre_save, sender=PlayerCharacterLink) -def link_presave_track_old_character(sender, instance, **kwargs): - """ - If someone edits a link and changes its character, we need to update BOTH: - the old character and the new character. - """ - instance._old_character_id = None - if instance.pk: - try: - old = PlayerCharacterLink.objects.only("character_id").get(pk=instance.pk) - instance._old_character_id = old.character_id - except PlayerCharacterLink.DoesNotExist: - pass - - -@receiver(post_save, sender=PlayerCharacterLink) -def link_postsave_recompute(sender, instance, **kwargs): - def _do(): - # update new/current character - recompute_character_flags(instance.character_id) - # update old character if link moved - old_id = getattr(instance, "_old_character_id", None) - if old_id and old_id != instance.character_id: - recompute_character_flags(old_id) - - transaction.on_commit(_do) - - -@receiver(post_delete, sender=PlayerCharacterLink) -def link_postdelete_recompute(sender, instance, **kwargs): - transaction.on_commit(lambda: recompute_character_flags(instance.character_id)) - - @receiver(post_delete, sender=CharacterRelationshipMembership) def delete_relationship_with_no_members(sender, instance, **kwargs): """ diff --git a/character/tests/test_filters.py b/character/tests/test_filters.py index de73f859..c90af8e1 100644 --- a/character/tests/test_filters.py +++ b/character/tests/test_filters.py @@ -17,7 +17,6 @@ def setUp(self): given_name="NPC1", birth_date=date(2000, 1, 1), sex="Male", - can_link=True, level=5, xp=100, ) @@ -25,7 +24,6 @@ def setUp(self): given_name="NPC2", birth_date=date(2000, 1, 1), sex="Female", - can_link=True, level=10, xp=500, ) @@ -45,7 +43,6 @@ def setUp(self): given_name="Player1", birth_date=date(2000, 1, 1), sex="Male", - can_link=False, level=3, xp=75, ) @@ -66,7 +63,6 @@ def setUp(self): given_name="Player2", birth_date=date(2000, 1, 1), sex="Female", - can_link=False, level=7, xp=200, ) diff --git a/character/tests/test_models.py b/character/tests/test_models.py index 45704cff..552717c4 100644 --- a/character/tests/test_models.py +++ b/character/tests/test_models.py @@ -413,13 +413,11 @@ def setUp(self): given_name="NPC1", birth_date=date(2000, 1, 1), sex="Male", - can_link=True, ) self.npc2 = Character.objects.create( given_name="NPC2", birth_date=date(2000, 1, 1), sex="Female", - can_link=True, ) # Create a player-linked character @@ -440,14 +438,10 @@ def setUp(self): given_name="Player", birth_date=date(2000, 1, 1), sex="Male", - can_link=False, ) PlayerCharacterLink.objects.create( player=self.player, character=self.player_character, is_active=True ) - # Update can_link to match real behavior - self.player_character.can_link = False - self.player_character.save() def test_is_npc_property_for_npc(self): """Test that a character without an active player link is an NPC""" @@ -483,8 +477,8 @@ def test_has_available_classmethod(self): def test_has_available_no_linkable_characters(self): """Test has_available returns False when no linkable characters exist""" - # Mark all NPCs as not linkable - Character.objects.filter(can_link=True).update(can_link=False) + # Mark all currently-linkable NPCs as reserved, so none remain linkable + Character.objects.linkable().update(is_reserved=True) self.assertFalse(Character.has_available()) def test_has_available_all_linked(self): @@ -506,65 +500,6 @@ def test_has_available_all_linked(self): self.assertFalse(Character.has_available()) -class PlayerCharacterLinkPointsTodayTests(TestCase): - """Tests for PlayerCharacterLink.player_time_today/points_today (issue #673).""" - - def setUp(self): - from progression.models import PlayerActivity - - self.PlayerActivity = PlayerActivity - self.user = user_factory(with_player=True) - self.player = self.user.player - character = Character.objects.create(given_name="Hero") - self.link = PlayerCharacterLink.objects.create( - player=self.player, character=character - ) - # Backdated well before "today" so player_time_today's max(start_of_day, - # linked_at) resolves to start_of_day in these tests, rather than to - # whatever moment setUp happened to run at. - self.link.linked_at = now() - timedelta(days=30) - self.link.save(update_fields=["linked_at"]) - - def _complete_activity(self, *, duration_seconds, completed_at): - return self.PlayerActivity.objects.create( - player=self.player, - is_complete=True, - duration=duration_seconds, - completed_at=completed_at, - ) - - def test_points_today_counts_only_activities_completed_today(self): - today_start = now().replace(hour=0, minute=0, second=0, microsecond=0) - self._complete_activity( - duration_seconds=1800, completed_at=today_start + timedelta(hours=2) - ) # 30 min today - self._complete_activity( - duration_seconds=3600, completed_at=today_start - timedelta(hours=1) - ) # 60 min yesterday - excluded - - self.assertEqual(self.link.player_time_today, 30) - self.assertEqual(self.link.points_today, 3) - - def test_points_today_excludes_activity_before_link_started(self): - today_start = now().replace(hour=0, minute=0, second=0, microsecond=0) - self.link.linked_at = today_start + timedelta(hours=5) - self.link.save(update_fields=["linked_at"]) - - self._complete_activity( - duration_seconds=1800, completed_at=today_start + timedelta(hours=1) - ) # today, but before the link started - excluded - self._complete_activity( - duration_seconds=600, completed_at=today_start + timedelta(hours=6) - ) # 10 min, after linked_at - - self.assertEqual(self.link.player_time_today, 10) - self.assertEqual(self.link.points_today, 1) - - def test_points_today_zero_with_no_activities(self): - self.assertEqual(self.link.player_time_today, 0) - self.assertEqual(self.link.points_today, 0) - - class CharacterTotalLinkPointsTests(TestCase): """Tests for Character.total_link_points (the character-side counterpart to Player.total_link_points).""" diff --git a/core/admin.py b/core/admin.py index 87c83e5b..bdc0cfc7 100644 --- a/core/admin.py +++ b/core/admin.py @@ -133,7 +133,12 @@ def has_add_permission(self, request): @admin.action(description="Publish selected announcements") def publish_selected_announcements(_modeladmin, _request, queryset): now = timezone.now() - queryset.update(is_published=True, published_at=now) + # Save individually (not queryset.update()) so Announcement.save() + # broadcasts the "announcement_published" WebSocket event per row. + for announcement in queryset: + announcement.is_published = True + announcement.published_at = now + announcement.save() @admin.action(description="Unpublish selected announcements") diff --git a/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py b/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py new file mode 100644 index 00000000..b30f306a --- /dev/null +++ b/core/migrations/0016_gamesettings_daily_goals_completion_bonus_ap.py @@ -0,0 +1,21 @@ +# Generated by Django 5.2.16 on 2026-08-10 00:00 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("core", "0015_featureflag"), + ] + + operations = [ + migrations.AddField( + model_name="gamesettings", + name="daily_goals_completion_bonus_ap", + field=models.IntegerField( + default=50, + help_text="Lump-sum AP awarded once per day when a player clears all three daily goals (see progression.daily_goals).", + ), + ), + ] diff --git a/core/models.py b/core/models.py index 07e9204c..0bb0ecff 100644 --- a/core/models.py +++ b/core/models.py @@ -90,6 +90,13 @@ class GameSettings(models.Model): max_digits=5, decimal_places=2, default="1.25" ) task_completion_xp = models.IntegerField(default=100) + daily_goals_completion_bonus_ap = models.IntegerField( + default=50, + help_text=( + "Lump-sum AP awarded once per day when a player clears all " + "three daily goals (see progression.daily_goals)." + ), + ) xp_mastery_scale = models.DecimalField( max_digits=10, decimal_places=2, @@ -162,6 +169,8 @@ def clean(self): errors["task_activity_xp_multiplier"] = "Must be > 0." if self.task_completion_xp < 0: errors["task_completion_xp"] = "Must be non-negative." + if self.daily_goals_completion_bonus_ap < 0: + errors["daily_goals_completion_bonus_ap"] = "Must be non-negative." if self.xp_mastery_scale <= 0: errors["xp_mastery_scale"] = "Must be > 0." if self.xp_mastery_multiplier_cap < 1: @@ -238,6 +247,33 @@ class Meta: def __str__(self): return self.title + def save(self, *args, **kwargs): + was_published = ( + Announcement.objects.filter(pk=self.pk, is_published=True).exists() + if self.pk + else False + ) + super().save(*args, **kwargs) + if self.is_published and not was_published: + from django.db import transaction + + transaction.on_commit(self._broadcast_published) + + def _broadcast_published(self): + from asgiref.sync import async_to_sync + + from gameplay.utils import send_group_message + + async_to_sync(send_group_message)( + "online_users", + { + "type": "action", + "action": "announcement_published", + "data": {"id": self.id}, + "success": True, + }, + ) + class PlayerAnnouncementState(models.Model): player = models.ForeignKey( diff --git a/frontend/.storybook/decorators/withAuthContext.tsx b/frontend/.storybook/decorators/withAuthContext.tsx new file mode 100644 index 00000000..9420aa0b --- /dev/null +++ b/frontend/.storybook/decorators/withAuthContext.tsx @@ -0,0 +1,19 @@ +import type { Decorator } from '@storybook/react-vite'; +import { AuthContext, type AuthContextValue } from '../../src/context/authContext'; +import { mockAuthContextValue } from '../../src/testUtils/mockAuthContext'; + +/** + * Wraps a story in `AuthContext` with a mock value, for components that read + * `useAuth()` outside of a real session. `authenticated` controls whether the + * mock represents a logged-in or logged-out user; `overrides` reaches any + * other field (e.g. `user: { is_staff: true }`). + */ +export function withAuthContext( + overrides: Partial & { authenticated?: boolean } = {} +): Decorator { + return (Story) => ( + + + + ); +} diff --git a/frontend/.storybook/decorators/withGameContext.tsx b/frontend/.storybook/decorators/withGameContext.tsx new file mode 100644 index 00000000..97591358 --- /dev/null +++ b/frontend/.storybook/decorators/withGameContext.tsx @@ -0,0 +1,14 @@ +import type { Decorator } from '@storybook/react-vite'; +import { GameContext } from '../../src/context/gameContext'; +import { mockGameContextValue } from '../../src/testUtils/mockGameContext'; + +/** + * Wraps a story in `GameContext` with `mockGameContextValue`, for components + * that read `useGame()` (directly, or transitively via `useFeatureFlag`) + * outside of a real game session. + */ +export const withGameContext: Decorator = (Story) => ( + + + +); diff --git a/frontend/.storybook/decorators/withQueryClient.tsx b/frontend/.storybook/decorators/withQueryClient.tsx new file mode 100644 index 00000000..d18fa097 --- /dev/null +++ b/frontend/.storybook/decorators/withQueryClient.tsx @@ -0,0 +1,27 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import type { Decorator } from '@storybook/react-vite'; + +/** + * Wraps a story in a fresh `QueryClient` per render, so components that call + * TanStack Query hooks (`useQuery`/`useQueryClient`) don't crash outside a + * provider. Retries are disabled - Storybook has no real API to fetch from, + * so a failed request should render its empty/error state immediately + * rather than retrying for several seconds. + * + * Seed data for a specific query (so a component renders populated instead + * of loading/empty) via a story's own decorator + `queryClient.setQueryData`, + * see `EntitySearchInput.stories.tsx` / `TutorialModal.stories.tsx`. + */ +export const withQueryClient: Decorator = (Story) => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false, staleTime: Infinity }, + }, + }); + + return ( + + + + ); +}; diff --git a/frontend/.storybook/preview.tsx b/frontend/.storybook/preview.tsx index 797871d2..ab69c54a 100644 --- a/frontend/.storybook/preview.tsx +++ b/frontend/.storybook/preview.tsx @@ -1,7 +1,13 @@ import type { Preview } from '@storybook/react-vite'; import '../src/styles/main.scss'; +import { withQueryClient } from './decorators/withQueryClient'; const preview: Preview = { + // Global so any component that calls a TanStack Query hook - directly or + // transitively (e.g. `useFeatureFlag` -> `useAppConfig`) - doesn't crash + // for lack of a QueryClientProvider ancestor. See withQueryClient's + // comment for how a story seeds its own query data. + decorators: [withQueryClient], parameters: { controls: { matchers: { diff --git a/frontend/src/api/activities.ts b/frontend/src/api/activities.ts index bbb97450..2d8a8019 100644 --- a/frontend/src/api/activities.ts +++ b/frontend/src/api/activities.ts @@ -1,7 +1,18 @@ // src/api/activities.ts import type { PlayerActivity } from "../types"; +import type { OfflineActivityLogResponse } from "../types/api"; import { apiFetch } from "../utils/api"; +export interface OfflineActivityLogPayload { + /** Name for a newly-created task; ignored (and optional) when `task` is set. */ + name?: string; + description?: string; + skill?: number; + task?: number; + started_at: string; + completed_at: string; +} + export function fetchActivities(): Promise { return (async () => { const allResults: PlayerActivity[] = []; @@ -45,3 +56,12 @@ export function deleteActivity(id: number): Promise { method: "DELETE", }); } + +export function logOfflineActivity( + data: OfflineActivityLogPayload +): Promise { + return apiFetch("/player-activities/log_offline/", { + method: "POST", + body: JSON.stringify(data), + }); +} diff --git a/frontend/src/api/map.ts b/frontend/src/api/map.ts index 49636b66..36b7b700 100644 --- a/frontend/src/api/map.ts +++ b/frontend/src/api/map.ts @@ -1,18 +1,24 @@ // src/api/map.ts import { apiFetch } from "../utils/api"; -interface PopulationCentreListItem { - id: number; +export interface InitialMapCentre { + id: number | null; + name: string | null; + // [minX, minY, maxX, maxY] in raw EPSG:3857 metres - just enough to frame + // the camera on this village; not the full per-village feature payload + // fetchPopulationCentreMap returns (see InitialMapCentreView's docstring). + bbox: [number, number, number, number] | null; } -type PopulationCentreListResponse = - | { results?: PopulationCentreListItem[] } - | PopulationCentreListItem[]; - -export async function fetchFirstPopulationCentreId(): Promise { - const data = await apiFetch("/population-centres/"); - const list = Array.isArray(data) ? data : (data?.results ?? []); - return list.length > 0 ? list[0].id : null; +// Which village the map's camera should open on - the requesting player's +// linked character's village if they have one, otherwise an arbitrary but +// deterministic fallback (see InitialMapCentreView, locations/views.py). +// Deliberately a separate, lightweight endpoint rather than reusing +// fetchPopulationCentreMap: this only needs to get the camera pointed at the +// right place before useMapViewport (bbox-scoped, polled) takes over as the +// source of truth moments later, once the camera's first move settles. +export function fetchInitialMapCentre(): Promise { + return apiFetch("/map/initial-centre/"); } export interface PopulationCentreSummary { diff --git a/frontend/src/api/player.ts b/frontend/src/api/player.ts index b4263b44..ba23d15b 100644 --- a/frontend/src/api/player.ts +++ b/frontend/src/api/player.ts @@ -41,13 +41,25 @@ export const deleteAccount = async (): Promise => { return response; }; -export interface TodayPointsResponse { - // null (not 0) when the player has no active PlayerCharacterLink - see - // MeViewSet.today_points in api/views.py. Callers use this to hide the - // map view's "today" badge entirely rather than showing a zero (issue #673). - points_today: number | null; +export interface DailyGoalsState { + logged_in_today: boolean; + completed_activity_today: boolean; + activity_minutes_today: number; + minutes_goal_threshold: number; + minutes_goal_met: boolean; + all_goals_met: boolean; + bonus_awarded_today: boolean; + bonus_ap: number; } -export const fetchTodayPoints = async (): Promise => { - return apiFetch("/me/today_points/"); +export interface DailyGoalsResponse { + // null (not a set of all-false goals) when the player has no active + // PlayerCharacterLink - see MeViewSet.daily_goals in api/views.py. + // Callers use this to hide the map view's badge entirely rather than + // showing an unearned "0 of 3" (issue #673, redesigned in #751). + goals: DailyGoalsState | null; +} + +export const fetchDailyGoals = async (): Promise => { + return apiFetch("/me/daily_goals/"); }; diff --git a/frontend/src/components/Achievements/Achievements.stories.tsx b/frontend/src/components/Achievements/Achievements.stories.tsx new file mode 100644 index 00000000..f748ff36 --- /dev/null +++ b/frontend/src/components/Achievements/Achievements.stories.tsx @@ -0,0 +1,93 @@ +import type { Meta, StoryObj } from '@storybook/react-vite'; +import Achievements from './Achievements'; + +/** + * `Achievements` renders a grid of achievement badges from a plain + * `achievements[]` prop - tier colour, progress bar, and the "time" vs. + * count value formatting are all derived from each achievement's fields. + */ +const meta: Meta = { + title: 'Shared/Achievements', + component: Achievements, + tags: ['autodocs'], +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + args: { + achievements: [ + { + type: 'tasks_completed', + label: 'Task Master', + symbol: '✅', + tier: 1, + complete: false, + color: 'grey', + value: 12, + threshold: 25, + }, + { + type: 'tasks_completed', + label: 'Task Master', + symbol: '✅', + tier: 2, + complete: true, + color: 'green', + value: 50, + threshold: 50, + }, + { + type: 'time', + label: 'Time Invested', + symbol: '⏱️', + tier: 3, + complete: false, + color: 'blue', + value: 5400, + threshold: 36000, + }, + { + type: 'streak', + label: 'Consistency', + symbol: '🔥', + tier: 4, + complete: false, + color: 'purple', + value: 8, + threshold: 30, + }, + { + type: 'level', + label: 'Levelled Up', + symbol: '⭐', + tier: 5, + complete: true, + color: 'gold', + value: 20, + threshold: 20, + }, + ], + }, +}; + +/** Every tier colour Achievements knows about (`grey`/`green`/`blue`/`purple`/`gold`), all mid-progress. */ +export const AllTiers: Story = { + args: { + achievements: (['grey', 'green', 'blue', 'purple', 'gold'] as const).map((color, i) => ({ + type: 'tasks_completed', + label: `Tier ${i + 1}`, + symbol: '🏅', + tier: i + 1, + complete: false, + color, + value: (i + 1) * 5, + threshold: 50, + })), + }, +}; + +export const Empty: Story = { + args: { achievements: [] }, +}; diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss index a1f3229b..520b000d 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.module.scss @@ -14,13 +14,37 @@ flex-direction: column; } -.dateTabs { +.toolbar { display: flex; + align-items: center; gap: sp.$spacing-sm; margin-bottom: sp.$spacing-md; + width: 100%; +} + +.dateTabs { + display: flex; + gap: sp.$spacing-sm; flex-wrap: wrap; justify-content: center; - width: 100%; + flex: 1; +} + +.addOfflineButton { + @include c.apply-button-variant(primary); + flex-shrink: 0; + border: none; + border-radius: 50%; + width: 2.25rem; + height: 2.25rem; + min-width: 2.25rem; + padding: 0; + font-size: 1.25rem; + line-height: 1; + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; } .dateButton { diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx index 1eea100b..99bfd72f 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.test.tsx @@ -2,8 +2,17 @@ import { render, screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TooltipProvider } from "../Tooltip/Tooltip"; import ActivitiesPanel from "./ActivitiesPanel"; +function renderActivitiesPanel() { + return render( + + + + ); +} + const mockUseActivities = vi.fn(); const mockUseDeleteActivity = vi.fn(); const mockUseUpdateActivity = vi.fn(); @@ -39,7 +48,7 @@ describe("ActivitiesPanel", () => { it("renders activities and delegates edit through PlayerItemList", async () => { const user = userEvent.setup(); - render(); + renderActivitiesPanel(); expect(screen.getByText("Write docs")).toBeInTheDocument(); @@ -47,19 +56,19 @@ describe("ActivitiesPanel", () => { const input = screen.getByLabelText("activity name"); await user.clear(input); await user.type(input, "Write tests"); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - activityId: 1, - data: { name: "Write tests" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { activityId: 1, data: { name: "Write tests" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); it("delegates delete confirmation through PlayerItemList", async () => { const user = userEvent.setup(); - render(); + renderActivitiesPanel(); await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); diff --git a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx index 614ce3ec..72e3c820 100644 --- a/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx +++ b/frontend/src/components/ActivitiesPanel/ActivitiesPanel.tsx @@ -6,6 +6,8 @@ import { formatDurationShort, pluralize } from "../../utils/formatUtils"; import Button from "../Button/Button"; import PlayerItemList from "../PlayerItemList/PlayerItemList"; +import Tooltip from "../Tooltip/Tooltip"; +import LogOfflineActivityModal from "../LogOfflineActivityModal/LogOfflineActivityModal"; import styles from "./ActivitiesPanel.module.scss"; type DateCategory = "today" | "yesterday" | "older"; @@ -68,6 +70,7 @@ export default function ActivitiesPanel(): React.ReactElement | null { const deleteActivity = useDeleteActivity(); const updateActivity = useUpdateActivity(); const [activeTab, setActiveTab] = useState("today"); + const [isLogModalOpen, setIsLogModalOpen] = useState(false); const bucketed = useMemo(() => bucketActivities(activities ?? []), [activities]); @@ -103,11 +106,14 @@ export default function ActivitiesPanel(): React.ReactElement | null { const hasTabActivities = groupedByCategory[activeTab].length > 0; const handleEdit = useCallback( - (activity: PlayerActivity, name: string) => { - updateActivity.mutate({ - activityId: activity.id, - data: { name }, - }); + (activity: PlayerActivity, name: string, callbacks?: { onSuccess?: () => void; onError?: () => void }) => { + updateActivity.mutate( + { + activityId: activity.id, + data: { name }, + }, + callbacks, + ); }, [updateActivity], ); @@ -124,8 +130,8 @@ export default function ActivitiesPanel(): React.ReactElement | null { return (
- {hasActivities && ( - <> +
+ {hasActivities && (
{dateTabs.map(({ key, label }) => (
+ )} + + + +
+ {isLogModalOpen && ( + setIsLogModalOpen(false)} /> + )} + + {hasActivities && ( + <> {hasTabActivities ? (
{Object.entries(activitiesByDay).map(([dateKey, dayActivities]) => { diff --git a/frontend/src/components/ActivityInput/ActivityInput.module.scss b/frontend/src/components/ActivityInput/ActivityInput.module.scss index 6a5a3d63..f4725474 100644 --- a/frontend/src/components/ActivityInput/ActivityInput.module.scss +++ b/frontend/src/components/ActivityInput/ActivityInput.module.scss @@ -4,7 +4,7 @@ @use '../../styles/semantic/spacing' as sp; @use 'sass:map'; -$row-control-height: 44px; +$row-control-height: sp.$form-control-height; .control { height: $row-control-height; display: flex; @@ -216,14 +216,6 @@ $row-control-height: 44px; } @include m.respond-to(md, down) { - .control { - height: 40px; - } - - .grow { - min-height: 40px; - } - .inputText { font-size: 1rem; line-height: 1.1; @@ -242,14 +234,6 @@ $row-control-height: 44px; } @include m.respond-to(sm, down) { - .control { - height: 38px; - } - - .grow { - min-height: 38px; - } - .inputText { font-size: 0.9rem; line-height: 1.05; diff --git a/frontend/src/components/ActivityInput/ActivityInput.tsx b/frontend/src/components/ActivityInput/ActivityInput.tsx index 89e77ae7..afb1eb4b 100644 --- a/frontend/src/components/ActivityInput/ActivityInput.tsx +++ b/frontend/src/components/ActivityInput/ActivityInput.tsx @@ -7,6 +7,7 @@ import EntitySearchInput from "../EntitySearchInput/EntitySearchInput"; import styles from "./ActivityInput.module.scss"; import { useActivityInput } from "./useActivityInput"; import SupportFlowModal from "../SupportFlow/SupportFlowModal"; +import { formatDuration } from "../../utils/formatUtils"; export default function ActivityInput() { const { @@ -14,8 +15,7 @@ export default function ActivityInput() { setName, isActive, inputValue, - minutes, - seconds, + elapsed, formattedLimit, showAutoStopWarning, flowState, @@ -63,7 +63,7 @@ export default function ActivityInput() {
- {minutes}:{seconds.toString().padStart(2, "0")} + {formatDuration(elapsed)}
+ +
+
+ + ); + } + + return ( + +
+
+ + Task * + + + {fieldErrors.task && ( +

{fieldErrors.task}

+ )} +
+ +
+ + Duration * + +
+ setDurationHours(value as string)} + className={styles.durationPartField} + /> + setDurationMinutes((value as string).slice(0, 2))} + maxLength={2} + className={styles.durationPartField} + /> +
+ {fieldErrors.duration && ( +

{fieldErrors.duration}

+ )} +
+ +
+ Completed +
+ handleCompletionDateChange(value as string)} + max={maxCompletionDate} + /> + handleCompletionTimeChange(value as string)} + /> +
+ {fieldErrors.completedAt && ( +

{fieldErrors.completedAt}

+ )} +
+ + {!Number.isNaN(totalDurationMinutes) && totalDurationMinutes > 0 && ( +

+ Logging {formatDurationShort(Math.round(totalDurationMinutes * 60))} +

+ )} + + {eligibility && ( +

+ {eligibility.message} +

+ )} + + {submitError && ( +

{submitError}

+ )} + +
+ + +
+
+ + ); +} diff --git a/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts b/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts new file mode 100644 index 00000000..03d24d1d --- /dev/null +++ b/frontend/src/components/LogOfflineActivityModal/useLogOfflineActivityForm.ts @@ -0,0 +1,242 @@ +import { useCallback, useMemo, useState } from "react"; + +import { useLogOfflineActivity } from "../../hooks/useActivities"; +import { useGame } from "../../hooks/useGame"; +import type { SearchEntity } from "../EntitySearchInput/useEntitySearchInput"; +import type { OfflineActivityLogResponse } from "../../types"; + +// Must match OFFLINE_XP_ELIGIBLE_BACKDATE_WINDOW in progression/services.py — +// duplicated here only for the client-side "will this earn XP" hint; the +// backend remains the authority on whether XP is actually awarded. +const XP_ELIGIBLE_BACKDATE_DAYS = 7; + +function pad(n: number): string { + return String(n).padStart(2, "0"); +} + +function todayDateValue(): string { + const now = new Date(); + return `${now.getFullYear()}-${pad(now.getMonth() + 1)}-${pad(now.getDate())}`; +} + +function nowTimeValue(): string { + const now = new Date(); + return `${pad(now.getHours())}:${pad(now.getMinutes())}`; +} + +function extractErrorMessage(error: unknown): string { + if (error instanceof Error && error.message) { + try { + const parsed = JSON.parse(error.message) as Record; + if (typeof parsed.message === "string") return parsed.message; + + const firstFieldError = Object.values(parsed).find( + (value): value is string[] => + Array.isArray(value) && typeof value[0] === "string" + ); + if (firstFieldError) return firstFieldError[0]; + } catch { + // Not a JSON error body — fall back to the raw message below. + } + return error.message; + } + return "Something went wrong logging this activity. Please try again."; +} + +export interface FieldErrors { + task?: string; + duration?: string; + completedAt?: string; +} + +export function useLogOfflineActivityForm(onLogged?: () => void) { + const { player, fetchPlayerAndCharacter } = useGame(); + const isPremium = Boolean(player?.is_premium); + const logOfflineActivity = useLogOfflineActivity(); + + const [taskName, setTaskName] = useState(""); + const [selectedTaskId, setSelectedTaskId] = useState(null); + const [durationHours, setDurationHours] = useState(""); + const [durationMinutes, setDurationMinutes] = useState(""); + const [completionDate, setCompletionDate] = useState(todayDateValue); + const [completionTime, setCompletionTime] = useState(nowTimeValue); + // Fixed at mount: caps the native date picker so future dates can't be + // selected at all, rather than allowing them and erroring afterwards. + const [maxCompletionDate] = useState(todayDateValue); + const [fieldErrors, setFieldErrors] = useState({}); + const [submitError, setSubmitError] = useState(null); + const [result, setResult] = useState(null); + // Read once at mount (a lazy initializer, not a render-phase call) and + // refreshed by the date/time change handlers below — those are event + // handlers, where reading the clock is fine. + const [nowMs, setNowMs] = useState(() => Date.now()); + + const handleCompletionDateChange = useCallback((value: string) => { + setCompletionDate(value); + setNowMs(Date.now()); + }, []); + + const handleCompletionTimeChange = useCallback((value: string) => { + setCompletionTime(value); + setNowMs(Date.now()); + }, []); + + const handleTaskNameChange = useCallback((value: string) => { + setTaskName(value); + setSelectedTaskId(null); + }, []); + + const handleTaskSelect = useCallback((entity: SearchEntity) => { + setTaskName(entity.name); + setSelectedTaskId(typeof entity.id === "number" ? entity.id : Number(entity.id)); + }, []); + + const totalDurationMinutes = useMemo(() => { + const hours = Number(durationHours || 0); + const minutes = Number(durationMinutes || 0); + if (Number.isNaN(hours) || Number.isNaN(minutes)) return NaN; + return hours * 60 + minutes; + }, [durationHours, durationMinutes]); + + const completedAt = useMemo(() => { + if (!completionDate || !completionTime) return null; + const date = new Date(`${completionDate}T${completionTime}`); + return Number.isNaN(date.getTime()) ? null : date; + }, [completionDate, completionTime]); + + const eligibility = useMemo(() => { + if (!completedAt) return null; + + if (!isPremium) { + return { + tone: "info" as const, + message: "This will be recorded, but offline activities only earn XP for Premium accounts.", + }; + } + + const ageDays = (nowMs - completedAt.getTime()) / (24 * 60 * 60 * 1000); + if (ageDays > XP_ELIGIBLE_BACKDATE_DAYS) { + return { + tone: "warning" as const, + message: `This is more than ${XP_ELIGIBLE_BACKDATE_DAYS} days old, so it will be recorded but won't earn XP.`, + }; + } + + return { + tone: "success" as const, + message: "This activity is eligible for XP, subject to your daily logging limits.", + }; + }, [completedAt, isPremium, nowMs]); + + const validate = useCallback((): FieldErrors => { + const errors: FieldErrors = {}; + + if (!taskName.trim()) { + errors.task = "Enter or select a task."; + } + + const hours = durationHours ? Number(durationHours) : 0; + const minutes = durationMinutes ? Number(durationMinutes) : 0; + if (Number.isNaN(hours) || hours < 0) { + errors.duration = "Hours must be a positive number."; + } else if (Number.isNaN(minutes) || minutes < 0 || minutes > 59) { + errors.duration = "Minutes must be between 0 and 59."; + } else if (totalDurationMinutes <= 0) { + errors.duration = "Enter a duration greater than zero."; + } + + if (!completionDate || !completionTime) { + errors.completedAt = "Enter a completion date and time."; + } else if (!completedAt) { + errors.completedAt = "Enter a valid completion date and time."; + } else if (completedAt.getTime() > Date.now()) { + errors.completedAt = "Completion date/time can't be in the future."; + } + + return errors; + }, [ + completedAt, + completionDate, + completionTime, + durationHours, + durationMinutes, + taskName, + totalDurationMinutes, + ]); + + const reset = useCallback(() => { + setTaskName(""); + setSelectedTaskId(null); + setDurationHours(""); + setDurationMinutes(""); + setCompletionDate(todayDateValue()); + setCompletionTime(nowTimeValue()); + setNowMs(Date.now()); + setFieldErrors({}); + setSubmitError(null); + setResult(null); + }, []); + + const handleSubmit = useCallback( + (event: React.FormEvent) => { + event.preventDefault(); + setSubmitError(null); + + const errors = validate(); + setFieldErrors(errors); + if (Object.keys(errors).length > 0 || !completedAt) return; + + const durationSeconds = Math.round(totalDurationMinutes * 60); + const startedAt = new Date(completedAt.getTime() - durationSeconds * 1000); + + logOfflineActivity.mutate( + { + ...(selectedTaskId ? { task: selectedTaskId } : { name: taskName.trim() }), + started_at: startedAt.toISOString(), + completed_at: completedAt.toISOString(), + }, + { + onSuccess: (data) => { + setResult(data); + if (data.xp_gained > 0) fetchPlayerAndCharacter(); + onLogged?.(); + }, + onError: (error) => setSubmitError(extractErrorMessage(error)), + } + ); + }, + [ + completedAt, + fetchPlayerAndCharacter, + logOfflineActivity, + onLogged, + selectedTaskId, + taskName, + totalDurationMinutes, + validate, + ] + ); + + return { + taskName, + handleTaskNameChange, + handleTaskSelect, + durationHours, + setDurationHours, + durationMinutes, + setDurationMinutes, + totalDurationMinutes, + completionDate, + handleCompletionDateChange, + completionTime, + handleCompletionTimeChange, + maxCompletionDate, + fieldErrors, + submitError, + result, + eligibility, + isSubmitting: logOfflineActivity.isPending, + handleSubmit, + reset, + }; +} diff --git a/frontend/src/components/Map/Map.test.tsx b/frontend/src/components/Map/Map.test.tsx index 33f39f40..c17e6d9c 100644 --- a/frontend/src/components/Map/Map.test.tsx +++ b/frontend/src/components/Map/Map.test.tsx @@ -1165,7 +1165,6 @@ describe('PopulationCentreMap entity detail card', () => { const dialog = await screen.findByRole('dialog', { name: 'House 2' }); expect(dialog).toHaveTextContent('1 / 4'); expect(dialog).toHaveTextContent('Alice'); - expect(dialog).toHaveTextContent('idle'); }); it('switches to a resident\'s own detail card when clicked inside the building detail card', async () => { diff --git a/frontend/src/components/Map/Map.tsx b/frontend/src/components/Map/Map.tsx index a901897e..6c6d296a 100644 --- a/frontend/src/components/Map/Map.tsx +++ b/frontend/src/components/Map/Map.tsx @@ -43,7 +43,12 @@ import { TOOLTIP_ONLY_SELECTION_OPACITY, VILLAGE_LABEL_LAYER, } from "./layers"; -import { buildVillageSourceData, type WalkerState } from "./sourceData"; +import { + buildCharacterPointFeatures, + buildStaticVillageFeatures, + buildVillageSourceData, + type WalkerState, +} from "./sourceData"; import MapDetailCard from "../MapDetailCard/MapDetailCard"; import CharacterDetail from "../CharacterDetail/CharacterDetail"; import BuildingDetail from "../BuildingDetail/BuildingDetail"; @@ -212,6 +217,17 @@ export default function PopulationCentreMap({ [features] ); + // Styled buildings/roads/fields/boundaries - everything the map draws + // except characters. Only recomputed when `features` itself changes (each + // ~2s poll), unlike character positions, which the walker loop below + // recomputes on every animation frame - keeping this out of that loop is + // what keeps a village's buildings/roads/fields from being re-styled and + // re-reprojected 60 times a second while nothing about them has changed. + const staticVillageFeatures = useMemo( + () => buildStaticVillageFeatures(features), + [features] + ); + // Lets scatterCharacters spread a field_shelter's idle workers across the // crops Subzone(s) it services instead of clustering them at the // shelter's own small footprint - see scatterCharacters' own comment. @@ -258,16 +274,19 @@ export default function PopulationCentreMap({ const walkersRef = useRef>(new Map()); const refreshVillageSource = useCallback(() => { - sourceRef.current?.setData( - buildVillageSourceData({ - features, - characterFeatures, - idleCharacterPositions, - walkers: walkersRef.current, - now: Date.now(), - }) - ); - }, [features, characterFeatures, idleCharacterPositions]); + sourceRef.current?.setData({ + type: "FeatureCollection", + features: [ + ...staticVillageFeatures, + ...buildCharacterPointFeatures({ + characterFeatures, + idleCharacterPositions, + walkers: walkersRef.current, + now: Date.now(), + }), + ], + }); + }, [staticVillageFeatures, characterFeatures, idleCharacterPositions]); // Creates the map once. onViewportChange and refreshVillageSource are each // read via a ref inside the handlers below rather than as effect deps, so @@ -689,18 +708,19 @@ export default function PopulationCentreMap({ // Each frame recomputes position from scratch - the checkpoint plus how // much time has passed since it was taken - rather than stepping forward // from wherever the previous frame left off, so nothing compounds across - // frames or across polls (see the WalkerState comment above). + // frames or across polls (see the WalkerState comment above). Only runs + // while at least one character actually has an active journey - an idle + // village (the common case) has nothing to animate, so there's no reason + // to keep a 60fps timer alive rebuilding the source every 16ms. useEffect(() => { - const step = () => { - if (mapReady) { - refreshVillageSource(); - } - }; + if (!mapReady || walkingFeatures.length === 0) return; + + const step = () => refreshVillageSource(); step(); const intervalId = window.setInterval(step, 16); return () => window.clearInterval(intervalId); - }, [mapReady, refreshVillageSource]); + }, [mapReady, walkingFeatures.length, refreshVillageSource]); // Outlines whichever building/character the detail card currently has // open (see SELECTED_BUILDING_OUTLINE_LAYER/SELECTED_CHARACTER_HIGHLIGHT_LAYER diff --git a/frontend/src/components/Map/geojson.tsx b/frontend/src/components/Map/geojson.tsx index a2960472..b5dc544a 100644 --- a/frontend/src/components/Map/geojson.tsx +++ b/frontend/src/components/Map/geojson.tsx @@ -111,6 +111,7 @@ export function polygonTooltipContent( ); } if (properties?.feature_type === "subzone") { + if (properties?.usage === "square") return "Square"; if (properties?.usage !== "crops") return properties?.name; const stage = properties?.crop_stage as string | null | undefined; @@ -125,6 +126,12 @@ export function polygonTooltipContent( return properties?.name; } +// Open communal outdoor space (see watabou_import._import_squares) - a +// warm, paved tone distinct from both a crops Subzone's green (fieldFillFor) +// and a building's default grey, so a plaza reads as open ground rather +// than a structure. +const SQUARE_FILL_COLOR = "#d8c9a8"; + // Precomputes per-feature presentation properties (fill/stroke) so map // styling can stay simple `["get", ...]` paint expressions instead of // duplicating fieldFillFor's stage/progress logic as a style expression. @@ -135,6 +142,8 @@ export function styledPolygonFeatures(features: GeoJSONFeature[]) { const isBoundary = f.properties?.feature_type === "boundary"; const isCropSubzone = f.properties?.feature_type === "subzone" && f.properties?.usage === "crops"; + const isSquareSubzone = + f.properties?.feature_type === "subzone" && f.properties?.usage === "square"; const fillColor = isBoundary ? "transparent" : isCropSubzone @@ -142,6 +151,8 @@ export function styledPolygonFeatures(features: GeoJSONFeature[]) { f.properties?.crop_stage as string | null | undefined, f.properties?.crop_progress as number | null | undefined ) + : isSquareSubzone + ? SQUARE_FILL_COLOR : "#ddd"; return { type: "Feature" as const, diff --git a/frontend/src/components/Map/sourceData.ts b/frontend/src/components/Map/sourceData.ts index 1a0da24e..45832b43 100644 --- a/frontend/src/components/Map/sourceData.ts +++ b/frontend/src/components/Map/sourceData.ts @@ -48,22 +48,37 @@ function positionAlongPath( return pos; } -interface BuildVillageSourceDataArgs { - features: GeoJSONFeature[]; +// Styled buildings/roads/fields/boundaries - everything except characters. +// This only changes when `features` itself changes (i.e. once per ~2s poll), +// unlike character positions, which are recomputed on every animation frame +// by the walker loop in Map.tsx. Callers should memoize this separately +// (keyed on `features`) rather than folding it into buildVillageSourceData, +// so that per-frame loop isn't re-styling and re-reprojecting every building/ +// road/field 60 times a second when only the characters are actually moving. +export function buildStaticVillageFeatures(features: GeoJSONFeature[]) { + return [ + ...styledPolygonFeatures(features), + ...styledLineFeatures(features), + ...styledPointFeatures( + features.filter((feature) => feature.properties?.feature_type !== "character") + ), + ]; +} + +interface BuildCharacterPointFeaturesArgs { characterFeatures: GeoJSONFeature[]; idleCharacterPositions: Map; walkers: Map; now: number; } -export function buildVillageSourceData({ - features, +export function buildCharacterPointFeatures({ characterFeatures, idleCharacterPositions, walkers, now, -}: BuildVillageSourceDataArgs) { - const characterPointFeatures: LngLatPointFeature[] = characterFeatures.map((feature) => { +}: BuildCharacterPointFeaturesArgs): LngLatPointFeature[] { + return characterFeatures.map((feature) => { const id = String(feature.properties?.id); const walker = walkers.get(id); const rawPoint = walker @@ -83,16 +98,34 @@ export function buildVillageSourceData({ properties: feature.properties, }; }); +} +interface BuildVillageSourceDataArgs { + features: GeoJSONFeature[]; + characterFeatures: GeoJSONFeature[]; + idleCharacterPositions: Map; + walkers: Map; + now: number; +} + +// Full rebuild of the source's FeatureCollection - static features plus +// current character positions. Used on mount and whenever `features` itself +// changes; the per-frame walker loop in Map.tsx calls +// buildCharacterPointFeatures directly against a memoized +// buildStaticVillageFeatures result instead, since that loop only ever needs +// to update character positions, not the static geometry around them. +export function buildVillageSourceData({ + features, + characterFeatures, + idleCharacterPositions, + walkers, + now, +}: BuildVillageSourceDataArgs) { return { type: "FeatureCollection" as const, features: [ - ...styledPolygonFeatures(features), - ...styledLineFeatures(features), - ...styledPointFeatures( - features.filter((feature) => feature.properties?.feature_type !== "character") - ), - ...characterPointFeatures, + ...buildStaticVillageFeatures(features), + ...buildCharacterPointFeatures({ characterFeatures, idleCharacterPositions, walkers, now }), ], }; } diff --git a/frontend/src/components/Modal/Modal.module.scss b/frontend/src/components/Modal/Modal.module.scss index 0ca97f77..6022ecb9 100644 --- a/frontend/src/components/Modal/Modal.module.scss +++ b/frontend/src/components/Modal/Modal.module.scss @@ -53,8 +53,7 @@ overflow-y: auto; overflow-x: hidden; min-height: 0; - padding-top: sp.$spacing-md; - padding-bottom: sp.$spacing-md; + padding: sp.$spacing-md sp.$spacing-sm; @include tc.two-column-layout; justify-content: flex-start; diff --git a/frontend/src/components/ModeSwitcher/ModeSwitcher.stories.tsx b/frontend/src/components/ModeSwitcher/ModeSwitcher.stories.tsx new file mode 100644 index 00000000..bcca2700 --- /dev/null +++ b/frontend/src/components/ModeSwitcher/ModeSwitcher.stories.tsx @@ -0,0 +1,45 @@ +import { useState } from 'react'; +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, userEvent } from 'storybook/test'; +import ModeSwitcher from './ModeSwitcher'; +import type { ModeOption } from './ModeSwitcher'; + +/** + * `ModeSwitcher` is a `radiogroup` of chips (e.g. Tasks/Activities panel + * mode). Fully generic over `modes`/`activeKey`/`onSelect`; arrow keys move + * (and select) between options, Home/End jump to the first/last. + */ +const modes: ModeOption[] = [ + { key: 'tasks', label: 'Tasks' }, + { key: 'activities', label: 'Activities' }, + { key: 'skills', label: 'Skills' }, +]; + +const meta: Meta = { + title: 'Shared/ModeSwitcher', + component: ModeSwitcher, + tags: ['autodocs'], + args: { + modes, + activeKey: 'tasks', + ariaLabel: 'View mode', + }, +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + render: (args) => { + function Wrapper() { + const [activeKey, setActiveKey] = useState(args.activeKey); + return ; + } + return ; + }, + play: async ({ canvas }) => { + const activities = canvas.getByRole('radio', { name: 'Activities' }); + await userEvent.click(activities); + await expect(activities).toHaveAttribute('aria-checked', 'true'); + }, +}; diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss index 87e9fd30..d577afcd 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.module.scss +++ b/frontend/src/components/PlayerItemList/PlayerItemList.module.scss @@ -156,19 +156,12 @@ } } -.childList { - list-style: none; - margin: sp.$spacing-xs 0 0; - padding: 0 0 0 sp.$spacing-lg; - display: flex; - flex-direction: column; - gap: sp.$spacing-xs; -} - .childItem { - background: transparent; - border-color: rgba(c.$color-border-primary, 0.12); - opacity: 0.9; + // Shrink width by the indent instead of just shifting it, so the right + // edge still lines up with the parent's — .listItem's width: 100% would + // otherwise push the box that far past the parent's right edge. + width: calc(100% - #{sp.$spacing-lg}); + margin-left: sp.$spacing-lg; } .itemCompleted { @@ -235,9 +228,10 @@ .editInput { flex: 1; min-width: 0; + height: sp.$form-control-height; padding: sp.$spacing-xs sp.$spacing-sm; border: 2px solid rgba(c.$color-status-success, 0.45); - border-radius: sp.$border-radius; + border-radius: sp.$form-control-radius; font-size: 1em; font-family: inherit; transition: border-color 0.2s ease; @@ -259,6 +253,7 @@ } .editConfirmActions { + position: relative; display: flex; justify-content: flex-start; align-items: center; @@ -290,3 +285,45 @@ color: rgba(0, 0, 0, 0.65); @include t.apply-text-style(t.$text-caption); } + +.saveStatus { + position: absolute; + right: 0; + top: 50%; + transform: translateY(-50%); + padding: sp.$spacing-xs sp.$spacing-sm; + border-radius: sp.$border-radius; + border: 1px solid rgba(c.$color-border-primary, 0.4); + background: rgba(c.$color-bg, 0.95); + box-shadow: 0 2px 8px rgba(0, 0, 0, 0.25); + pointer-events: none; + z-index: 1; + font-weight: 600; + @include t.apply-text-style(t.$text-caption); + animation: saveStatusFadeIn 0.15s ease; +} + +.saveStatus-saving { + color: rgba(0, 0, 0, 0.6); +} + +.saveStatus-saved { + color: c.$color-status-success; + animation: saveStatusFadeInOut #{2500ms} ease forwards; +} + +.saveStatus-error { + color: c.$color-status-danger; +} + +@keyframes saveStatusFadeIn { + from { opacity: 0; } + to { opacity: 1; } +} + +@keyframes saveStatusFadeInOut { + 0% { opacity: 0; } + 10% { opacity: 1; } + 80% { opacity: 1; } + 100% { opacity: 0; } +} diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.stories.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.stories.tsx new file mode 100644 index 00000000..a4edf20b --- /dev/null +++ b/frontend/src/components/PlayerItemList/PlayerItemList.stories.tsx @@ -0,0 +1,111 @@ +import type { Meta, StoryObj } from '@storybook/react-vite'; +import { expect, userEvent, within } from 'storybook/test'; +import PlayerItemList from './PlayerItemList'; +import type { FilterOption, SortOption } from './PlayerItemList'; + +/** + * `PlayerItemList` is the generic, prop-driven list underlying + * `CategoriesPanel`, `ProjectsPanel`, `SkillsPanel`, `TasksPanel`, and + * `ActivitiesPanel` - it owns sorting, filtering, the edit/delete modal, + * hover-edit affordances, and nested children, while the panel supplies the + * item shape and callbacks. Storying it here documents most of what those + * five panels visually do. + */ +interface DemoItem { + id: number; + name: string; + detail: string; + complete?: boolean; +} + +const items: DemoItem[] = [ + { id: 1, name: 'Write docs', detail: 'Duration: 15m · 15 XP gained', complete: false }, + { id: 2, name: 'Fix login bug', detail: 'Duration: 42m · 40 XP gained', complete: true }, + { id: 3, name: 'Plan sprint', detail: 'Duration: 5m · 5 XP gained', complete: false }, +]; + +const sortOptions: SortOption[] = [ + { key: 'name', label: 'Name', compareFn: (a, b) => a.name.localeCompare(b.name) }, + { key: 'newest', label: 'Newest', compareFn: (a, b) => b.id - a.id }, +]; + +const filterOptions: FilterOption[] = [ + { key: 'all', label: 'All', predicate: () => true }, + { key: 'incomplete', label: 'Incomplete', predicate: (item) => !item.complete }, +]; + +const meta: Meta> = { + title: 'Shared/PlayerItemList', + component: PlayerItemList, + tags: ['autodocs'], + args: { + items, + itemLabel: 'activity', + ariaLabel: 'Activities', + renderItemMeta: (item: DemoItem) => item.detail, + onEdit: () => {}, + }, +}; + +export default meta; +type Story = StoryObj>; + +export const Default: Story = {}; + +/** `hoverEdit` swaps the row's click-to-open button for a persistent detail area plus a hover-revealed edit icon - used by panels where the row itself does something else on click. */ +export const HoverEdit: Story = { + args: { hoverEdit: true }, +}; + +/** `isItemComplete`/`onToggleComplete` add a per-row checkbox, e.g. for `TasksPanel`. */ +export const WithCompleteToggle: Story = { + args: { + isItemComplete: (item: DemoItem) => Boolean(item.complete), + onToggleComplete: () => {}, + }, +}; + +/** `sortOptions`/`filterOptions` render a controls bar above the list. */ +export const WithSortAndFilter: Story = { + args: { sortOptions, filterOptions }, + play: async ({ canvas }) => { + await expect(canvas.getByRole('group', { name: 'Filter activitys' })).toBeVisible(); + await expect(canvas.getByLabelText('Sort:')).toBeVisible(); + }, +}; + +/** `getChildren` nests an item's children directly under it (e.g. subtasks), independent of the active sort/filter. */ +export const WithNestedChildren: Story = { + args: { + items: [ + { id: 1, name: 'Ship v1', detail: '2 subtasks', complete: false }, + { id: 2, name: 'Write changelog', detail: 'Duration: 10m', complete: false }, + { id: 3, name: 'Cut release', detail: 'Duration: 5m', complete: true }, + ], + getChildren: (item: DemoItem) => + item.id === 1 + ? [ + { id: 2, name: 'Write changelog', detail: 'Duration: 10m', complete: false }, + { id: 3, name: 'Cut release', detail: 'Duration: 5m', complete: true }, + ] + : undefined, + }, +}; + +/** No items - the shared empty list state (an empty `
    `, no placeholder copy of its own). */ +export const Empty: Story = { + args: { items: [] }, +}; + +/** Clicking a row opens the edit modal; `onDelete` adds a Delete action with its own confirm step. */ +export const EditModal: Story = { + args: { onDelete: () => {} }, + play: async ({ canvasElement, canvas }) => { + await userEvent.click(canvas.getByRole('button', { name: 'Open activity Write docs' })); + + const body = within(canvasElement.ownerDocument.body); + const dialog = await body.findByRole('dialog', { name: 'Edit activity' }); + await expect(dialog).toBeVisible(); + await expect(body.getByRole('button', { name: 'Delete' })).toBeVisible(); + }, +}; diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx index 682abdd2..6142c2df 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.test.tsx @@ -26,7 +26,7 @@ describe("PlayerItemList", () => { expect(screen.getByText("Duration: 15m • 15 XP gained")).toBeInTheDocument(); }); - it("opens the edit modal and saves a trimmed name", async () => { + it("autosaves a trimmed name on blur", async () => { const user = userEvent.setup(); const onEdit = vi.fn(); @@ -43,10 +43,57 @@ describe("PlayerItemList", () => { const input = screen.getByLabelText("activity name"); await user.clear(input); await user.type(input, " Deep work "); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); - expect(onEdit).toHaveBeenCalledWith(items[0], "Deep work"); - expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); + expect(onEdit).toHaveBeenCalledWith( + items[0], + "Deep work", + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); + // Autosave doesn't close the modal — there's no explicit Save action anymore. + expect(screen.getByRole("dialog")).toBeInTheDocument(); + }); + + it("does not re-fire the save when blurring without changing the name", async () => { + const user = userEvent.setup(); + const onEdit = vi.fn(); + + render( + , + ); + + await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); + const input = screen.getByLabelText("activity name"); + await user.click(input); + await user.tab(); + + expect(onEdit).not.toHaveBeenCalled(); + }); + + it("shows a Saved indicator once the autosave succeeds, then shows a warning on failure", async () => { + const user = userEvent.setup(); + const onEdit = vi.fn((_item, _name, callbacks) => callbacks?.onSuccess?.()); + + const { rerender } = render( + , + ); + + await user.click(screen.getByRole("button", { name: "Open activity Write docs" })); + const input = screen.getByLabelText("activity name"); + await user.clear(input); + await user.type(input, "Deep work"); + await user.tab(); + + expect(await screen.findByText("Saved")).toBeInTheDocument(); + + const failingOnEdit = vi.fn((_item, _name, callbacks) => callbacks?.onError?.()); + rerender(); + + await user.clear(input); + await user.type(input, "Deeper work"); + await user.tab(); + + expect(await screen.findByText("Couldn't save — try again")).toBeInTheDocument(); }); it("opens the delete modal and confirms deletion", async () => { @@ -154,7 +201,7 @@ describe("PlayerItemList", () => { const child = { id: 11, name: "Child task" }; const flatItems = [parent, child]; - it("renders children nested under their parent without a top-level row", () => { + it("renders children as independent rows, indented, directly after their parent", () => { render( { expect(screen.getByRole("button", { name: "Open task Parent task" })).toBeInTheDocument(); expect(screen.getByRole("button", { name: "Open task Child task" })).toBeInTheDocument(); - // The child is not rendered as its own top-level list item. + // The child is not duplicated as a second top-level entry sourced from `items`. expect(screen.getAllByRole("button", { name: /^Open task / })).toHaveLength(2); + + // Both render as siblings within the list, not one nested inside the other. + const rows = screen.getAllByRole("listitem"); + expect(rows).toHaveLength(2); + expect(rows[0]).toHaveTextContent("Parent task"); + expect(rows[1]).toHaveTextContent("Child task"); + expect(within(rows[0]).queryByText("Child task")).not.toBeInTheDocument(); }); it("is unaffected when getChildren is not passed (ProjectsPanel-style usage)", () => { diff --git a/frontend/src/components/PlayerItemList/PlayerItemList.tsx b/frontend/src/components/PlayerItemList/PlayerItemList.tsx index 53356cc3..0617d707 100644 --- a/frontend/src/components/PlayerItemList/PlayerItemList.tsx +++ b/frontend/src/components/PlayerItemList/PlayerItemList.tsx @@ -1,12 +1,14 @@ -import React, { useEffect, useMemo } from "react"; +import React, { useCallback, useEffect, useMemo } from "react"; import classNames from "classnames"; import Button from "../Button/Button"; import List from "../List/List"; -import Li from "../List/Li"; import Modal from "../Modal/Modal"; +import SaveStatusIndicator from "./SaveStatusIndicator"; import { usePlayerItemListControls } from "./usePlayerItemListControls"; import { usePlayerItemModal } from "./usePlayerItemModal"; +import type { SaveCallbacks } from "./usePlayerItemModal"; +import type { SaveStatusHelpers } from "./useSaveStatus"; import styles from "./PlayerItemList.module.scss"; export interface SortOption { @@ -30,8 +32,10 @@ interface PlayerItemListProps onToggleComplete?: (item: T) => void; getItemKey?: (item: T, index: number) => string | number; renderItemMeta?: (item: T) => React.ReactNode; - renderEditSummary?: (item: T) => React.ReactNode; - onEdit?: (item: T, name: string) => void; + renderEditSummary?: (item: T, saveHelpers: SaveStatusHelpers) => React.ReactNode; + /** Rendered next to the name input in the edit modal's title row (e.g. an icon button). */ + renderTitleRowActions?: (item: T) => React.ReactNode; + onEdit?: (item: T, name: string, callbacks?: SaveCallbacks) => void; onDelete?: (item: T) => void; hoverEdit?: boolean; renderRowActions?: (item: T) => React.ReactNode; @@ -45,6 +49,10 @@ interface PlayerItemListProps /** Called once the requested `openItemId` has been opened, so the caller can clear it. */ onOpenItemHandled?: () => void; getChildren?: (item: T) => T[] | undefined; + /** Ids of items present in `items` (e.g. for the deep-link lookup) that should not be rendered as rows. */ + hiddenItemIds?: Set; + /** Called with the item whose edit modal just closed (via Close, backdrop, or Escape). */ + onModalClose?: (item: T) => void; } export default function PlayerItemList({ @@ -57,6 +65,7 @@ export default function PlayerItemList) { const { activeFilterKey, @@ -92,6 +103,7 @@ export default function PlayerItemList { + if (activeItem) onModalClose?.(activeItem); + handleModalClose(); + }, [activeItem, onModalClose, handleModalClose]); + const canToggleComplete = typeof onToggleComplete === "function"; const canEdit = typeof onEdit === "function"; const canDelete = typeof onDelete === "function"; @@ -137,12 +156,23 @@ export default function PlayerItemList { - if (!getChildren) return displayItems; - return displayItems.filter( + // Items present in `items` only so the deep-link/openItemId lookup can find + // them (e.g. an unsaved draft) are excluded from the rendered rows. + const visibleDisplayItems = useMemo(() => { + if (!hiddenItemIds || hiddenItemIds.size === 0) return displayItems; + return displayItems.filter((item) => item.id === undefined || !hiddenItemIds.has(item.id)); + }, [displayItems, hiddenItemIds]); + + // Sort/filter controls only apply to top-level items; a child keeps its + // place directly after its parent (in `getChildren`'s order) rather than + // being reordered independently. + const flatDisplayItems = useMemo(() => { + if (!getChildren) return visibleDisplayItems; + const topLevel = visibleDisplayItems.filter( (item) => item.id === undefined || !childIds.has(item.id) ); - }, [displayItems, getChildren, childIds]); + return topLevel.flatMap((item) => [item, ...(getChildren(item) ?? [])]); + }, [visibleDisplayItems, getChildren, childIds]); const renderRow = (item: T): React.ReactNode => ( <> @@ -252,7 +282,7 @@ export default function PlayerItemList classNames(styles.item, { + [styles.childItem]: item.id !== undefined && childIds.has(item.id), [styles.itemCompleted]: isItemComplete?.(item), }) } - renderItem={(item) => { - const children = getChildren?.(item); - return ( - <> - {renderRow(item)} - {children?.length ? ( -
      - {children.map((child, index) => ( -
    • - {renderRow(child)} -
      • - ))} -
    - ) : null} - - ); - }} + renderItem={(item) => renderRow(item)} /> @@ -296,7 +306,7 @@ export default function PlayerItemList setConfirmingDelete(false) : undefined} backLabel="Back" > @@ -337,32 +347,32 @@ export default function PlayerItemList setEditingName(event.target.value)} + onBlur={handleEditSave} autoFocus onKeyDown={(event) => { if (event.key === "Enter") handleEditSave(); - if (event.key === "Escape") handleModalClose(); + if (event.key === "Escape") closeModal(); }} /> ) : null} + {renderTitleRowActions && liveActiveItem + ? renderTitleRowActions(liveActiveItem) + : null} ) : null} {modalSummary ? (
    {modalSummary}
    ) : null}
    - {canEdit ? ( - - ) : null} - {canDelete ? ( ) : null} +
    )} diff --git a/frontend/src/components/PlayerItemList/SaveStatusIndicator.tsx b/frontend/src/components/PlayerItemList/SaveStatusIndicator.tsx new file mode 100644 index 00000000..abe08832 --- /dev/null +++ b/frontend/src/components/PlayerItemList/SaveStatusIndicator.tsx @@ -0,0 +1,27 @@ +import classNames from "classnames"; +import type { SaveStatus } from "./useSaveStatus"; +import styles from "./PlayerItemList.module.scss"; + +interface SaveStatusIndicatorProps { + status: SaveStatus; +} + +const STATUS_TEXT: Record, string> = { + saving: "Saving…", + saved: "Saved", + error: "Couldn't save — try again", +}; + +export default function SaveStatusIndicator({ status }: SaveStatusIndicatorProps) { + if (status === "idle") return null; + + return ( +
    + {STATUS_TEXT[status]} +
    + ); +} diff --git a/frontend/src/components/PlayerItemList/usePlayerItemModal.ts b/frontend/src/components/PlayerItemList/usePlayerItemModal.ts index 8884a21f..4e552cda 100644 --- a/frontend/src/components/PlayerItemList/usePlayerItemModal.ts +++ b/frontend/src/components/PlayerItemList/usePlayerItemModal.ts @@ -1,11 +1,18 @@ -import { useCallback, useMemo, useState } from "react"; +import { useCallback, useMemo, useRef, useState } from "react"; + +import { useSaveStatus, type SaveStatusHelpers } from "./useSaveStatus"; + +export interface SaveCallbacks { + onSuccess?: () => void; + onError?: () => void; +} interface UsePlayerItemModalProps { items: T[]; getItemName: (item: T) => string; renderItemMeta?: (item: T) => React.ReactNode; - renderEditSummary?: (item: T) => React.ReactNode; - onEdit?: (item: T, name: string) => void; + renderEditSummary?: (item: T, saveHelpers: SaveStatusHelpers) => React.ReactNode; + onEdit?: (item: T, name: string, callbacks?: SaveCallbacks) => void; onDelete?: (item: T) => void; } @@ -20,34 +27,55 @@ export function usePlayerItemModal(null); const [editingName, setEditingName] = useState(""); const [confirmingDelete, setConfirmingDelete] = useState(false); + // Tracks the last name we've committed (or started committing) a save + // for, so blur/Enter only fire a request when the name actually changed. + const lastSavedNameRef = useRef(""); + + const { saveStatus, reportSaving, reportSaved, reportError, resetSaveStatus } = useSaveStatus(); const handleOpenItem = useCallback( (item: T) => { setActiveItem(item); - setEditingName(getItemName(item)); + const name = getItemName(item); + setEditingName(name); + lastSavedNameRef.current = name; + resetSaveStatus(); }, - [getItemName], + [getItemName, resetSaveStatus], ); const handleModalClose = useCallback(() => { setActiveItem(null); setEditingName(""); setConfirmingDelete(false); - }, []); + resetSaveStatus(); + }, [resetSaveStatus]); - const handleEditSave = useCallback(() => { + // Autosaves the name field. Fires on blur (and Enter) rather than on + // every keystroke, mirroring the due-date field's commit-on-blur pattern. + const commitNameEdit = useCallback(() => { if (!activeItem || !onEdit) { return; } const trimmedName = editingName.trim(); - if (!trimmedName) { + if (!trimmedName || trimmedName === lastSavedNameRef.current) { return; } - onEdit(activeItem, trimmedName); - handleModalClose(); - }, [activeItem, editingName, onEdit, handleModalClose]); + lastSavedNameRef.current = trimmedName; + reportSaving(); + onEdit(activeItem, trimmedName, { + onSuccess: reportSaved, + onError: reportError, + }); + }, [activeItem, editingName, onEdit, reportSaving, reportSaved, reportError]); + + // handleEditSave is kept as the exported name for the commit action + // (bound to the name input's blur/Enter handlers) since it's still the + // thing that saves the edit — there's just no explicit "Save" button + // triggering it anymore. + const handleEditSave = commitNameEdit; const handleDeleteRequest = useCallback(() => { setConfirmingDelete(true); @@ -82,23 +110,29 @@ export function usePlayerItemModal( + () => ({ reportSaving, reportSaved, reportError }), + [reportSaving, reportSaved, reportError], + ); + const modalSummary = useMemo(() => { if (!liveActiveItem) { return null; } return ( - renderEditSummary?.(liveActiveItem) ?? + renderEditSummary?.(liveActiveItem, saveHelpers) ?? renderItemMeta?.(liveActiveItem) ?? null ); - }, [liveActiveItem, renderEditSummary, renderItemMeta]); + }, [liveActiveItem, renderEditSummary, renderItemMeta, saveHelpers]); return { activeItem, liveActiveItem, editingName, confirmingDelete, + saveStatus, activeItemName, modalSummary, diff --git a/frontend/src/components/PlayerItemList/useSaveStatus.ts b/frontend/src/components/PlayerItemList/useSaveStatus.ts new file mode 100644 index 00000000..077f03da --- /dev/null +++ b/frontend/src/components/PlayerItemList/useSaveStatus.ts @@ -0,0 +1,77 @@ +import { useCallback, useEffect, useRef, useState } from "react"; + +export type SaveStatus = "idle" | "saving" | "saved" | "error"; + +export interface SaveStatusHelpers { + reportSaving: () => void; + reportSaved: () => void; + reportError: () => void; +} + +const SAVED_DISPLAY_MS = 2500; +// Saves usually resolve fast enough that "Saving…" would just flash on +// screen; only show it once a save has been pending this long. +const SAVING_DISPLAY_DELAY_MS = 150; + +/** + * Tracks a single in-flight autosave's status so a modal can show a brief + * "Saved"/"Couldn't save" indicator. "saved" auto-clears after a few + * seconds; "error" persists until the next save attempt or a manual reset, + * since a missed save is worse than a lingering banner. + */ +export function useSaveStatus() { + const [saveStatus, setSaveStatus] = useState("idle"); + const hideTimerRef = useRef | null>(null); + const savingTimerRef = useRef | null>(null); + + const clearHideTimer = useCallback(() => { + if (hideTimerRef.current) { + clearTimeout(hideTimerRef.current); + hideTimerRef.current = null; + } + }, []); + + const clearSavingTimer = useCallback(() => { + if (savingTimerRef.current) { + clearTimeout(savingTimerRef.current); + savingTimerRef.current = null; + } + }, []); + + const reportSaving = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + savingTimerRef.current = setTimeout(() => { + savingTimerRef.current = null; + setSaveStatus("saving"); + }, SAVING_DISPLAY_DELAY_MS); + }, [clearHideTimer, clearSavingTimer]); + + const reportSaved = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + setSaveStatus("saved"); + hideTimerRef.current = setTimeout(() => setSaveStatus("idle"), SAVED_DISPLAY_MS); + }, [clearHideTimer, clearSavingTimer]); + + const reportError = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + setSaveStatus("error"); + }, [clearHideTimer, clearSavingTimer]); + + const resetSaveStatus = useCallback(() => { + clearHideTimer(); + clearSavingTimer(); + setSaveStatus("idle"); + }, [clearHideTimer, clearSavingTimer]); + + useEffect(() => { + return () => { + clearHideTimer(); + clearSavingTimer(); + }; + }, [clearHideTimer, clearSavingTimer]); + + return { saveStatus, reportSaving, reportSaved, reportError, resetSaveStatus }; +} diff --git a/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx b/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx index 2adb9039..be0a916f 100644 --- a/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx +++ b/frontend/src/components/ProjectsPanel/ProjectsPanel.test.tsx @@ -77,16 +77,16 @@ describe("ProjectsPanel", () => { const input = screen.getByLabelText("project name"); await user.clear(input); await user.type(input, "Platform refresh"); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { name: "Platform refresh" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { name: "Platform refresh" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); - await user.click(screen.getByRole("button", { name: "Open project Website overhaul" })); + // The modal stays open after an autosave — no explicit Save click to close it. await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Delete" })); diff --git a/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx b/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx index 50d3b3ec..a927c541 100644 --- a/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx +++ b/frontend/src/components/SkillsPanel/SkillsPanel.test.tsx @@ -44,13 +44,13 @@ describe("SkillsPanel", () => { const input = screen.getByLabelText("skill name"); await user.clear(input); await user.type(input, "Research"); - await user.click(screen.getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { name: "Research" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { name: "Research" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); diff --git a/frontend/src/components/StaticBanner/StaticBanner.stories.tsx b/frontend/src/components/StaticBanner/StaticBanner.stories.tsx new file mode 100644 index 00000000..96e31d40 --- /dev/null +++ b/frontend/src/components/StaticBanner/StaticBanner.stories.tsx @@ -0,0 +1,25 @@ +import type { Meta, StoryObj } from '@storybook/react-vite'; +import StaticBanner from './StaticBanner'; + +/** + * `StaticBanner` is a single-line site announcement. It renders nothing + * when `message` is empty/unset, so a running app with no announcement + * configured shows no banner at all. + */ +const meta: Meta = { + title: 'Shared/StaticBanner', + component: StaticBanner, + tags: ['autodocs'], +}; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { + args: { message: "Scheduled maintenance tonight at 10pm UTC - the app may be briefly unavailable." }, +}; + +/** No `message` - renders nothing. */ +export const NoMessage: Story = { + args: {}, +}; diff --git a/frontend/src/components/TasksPanel/TasksPanel.module.scss b/frontend/src/components/TasksPanel/TasksPanel.module.scss index ca842fd1..b38d1f24 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.module.scss +++ b/frontend/src/components/TasksPanel/TasksPanel.module.scss @@ -99,6 +99,27 @@ gap: sp.$spacing-sm; } +.timestampButton { + flex-shrink: 0; + height: sp.$form-control-height; + width: sp.$form-control-height; + padding: 0; + border: 1px solid rgba(c.$color-border-primary, 0.35); + border-radius: sp.$form-control-radius; + background: transparent; + color: inherit; + font-size: 1rem; + line-height: 1; + display: inline-flex; + align-items: center; + justify-content: center; + cursor: pointer; + + &:hover { + border-color: rgba(c.$color-border-primary, 0.55); + } +} + .timestampLabel { font-weight: 600; margin-bottom: 2px; diff --git a/frontend/src/components/TasksPanel/TasksPanel.test.tsx b/frontend/src/components/TasksPanel/TasksPanel.test.tsx index 5671697c..a503b5cf 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.test.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.test.tsx @@ -284,13 +284,13 @@ describe("TasksPanel", () => { const input = screen.getByLabelText("task name"); await user.clear(input); await user.type(input, "Evening routine"); - await user.click(within(screen.getByRole("dialog")).getByRole("button", { name: "Save" })); + await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { name: "Evening routine" }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { name: "Evening routine" } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); @@ -325,7 +325,7 @@ describe("TasksPanel", () => { total_records: 0, }; - it("renders a subtask nested under its parent", () => { + it("renders a subtask as an independent, indented row directly after its parent", () => { mockUseTasks.mockReturnValue({ isLoading: false, data: [parentTask, childTask], @@ -336,9 +336,14 @@ describe("TasksPanel", () => { const childButton = screen.getAllByRole("button", { name: "Edit task Child subtask" })[0]; expect(parentButton).toBeInTheDocument(); expect(childButton).toBeInTheDocument(); - // The subtask is nested inside the parent's
  • , not a sibling top-level row. + // The subtask is its own sibling row, not nested inside the parent's
  • . const parentListItem = parentButton.closest("li"); - expect(parentListItem).toContainElement(childButton); + const childListItem = childButton.closest("li"); + expect(parentListItem).not.toBe(childListItem); + expect(parentListItem).not.toContainElement(childButton); + + const rows = screen.getAllByRole("listitem"); + expect(rows.indexOf(childListItem!)).toBe(rows.indexOf(parentListItem!) + 1); }); it("hides a completed parent and its subtasks together", () => { @@ -354,7 +359,7 @@ describe("TasksPanel", () => { expect(screen.queryByText("Child subtask")).not.toBeInTheDocument(); }); - it("pre-fills the add-task form with a parent chip via the add-subtask row action", async () => { + it("opens the task detail modal for a blank draft subtask without creating one yet", async () => { const user = userEvent.setup({ pointerEventsCheck: 0 }); mockUseTasks.mockReturnValue({ isLoading: false, @@ -364,15 +369,65 @@ describe("TasksPanel", () => { await user.click(screen.getByRole("button", { name: "Add subtask to Parent project task" })); - expect(screen.getByText(/Subtask of Parent project task/)).toBeInTheDocument(); + const dialog = await screen.findByRole("dialog"); + expect(within(dialog).getByLabelText("task name")).toHaveValue(""); + expect(createMutate).not.toHaveBeenCalled(); + }); + + it("creates the subtask only once its draft name has actually been edited, then opens the persisted task", async () => { + const user = userEvent.setup({ pointerEventsCheck: 0 }); + const newSubtask = { ...childTask, id: 7, name: "New task" }; + createMutate.mockImplementation((_data, callbacks) => { + callbacks?.onSuccess?.(newSubtask); + }); + mockUseTasks.mockReturnValue({ + isLoading: false, + data: [parentTask], + }); + const { rerender } = renderTasksPanel(); + + await user.click(screen.getByRole("button", { name: "Add subtask to Parent project task" })); + const dialog = await screen.findByRole("dialog"); + const input = within(dialog).getByLabelText("task name"); + await user.type(input, "New task"); + await user.tab(); + + expect(createMutate).toHaveBeenCalledWith( + { name: "New task", parent: 3 }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); + + // The new subtask isn't in `items` until the tasks query refetches with it included. + mockUseTasks.mockReturnValue({ + isLoading: false, + data: [parentTask, newSubtask], + }); + rerender( + + + , + ); + + const reopenedDialog = await screen.findByRole("dialog"); + expect(within(reopenedDialog).getByDisplayValue("New task")).toBeInTheDocument(); + }); + + it("discards the draft subtask, without creating anything, when its modal is closed unedited", async () => { + const user = userEvent.setup({ pointerEventsCheck: 0 }); + mockUseTasks.mockReturnValue({ + isLoading: false, + data: [parentTask], + }); + renderTasksPanel(); - const input = screen.getByLabelText("new task"); - await user.type(input, "Buy groceries"); - await user.click(screen.getByRole("button", { name: "Add subtask" })); + await user.click(screen.getByRole("button", { name: "Add subtask to Parent project task" })); + const dialog = await screen.findByRole("dialog"); + await user.click(within(dialog).getByRole("button", { name: "Close" })); await waitFor(() => { - expect(createMutate).toHaveBeenCalledWith({ name: "Buy groceries", parent: 3 }); + expect(screen.queryByRole("dialog")).not.toBeInTheDocument(); }); + expect(createMutate).not.toHaveBeenCalled(); }); it("disables the parent picker for a task that already has subtasks", async () => { @@ -401,15 +456,61 @@ describe("TasksPanel", () => { ); const dueDateInput = screen.getByLabelText("Due date"); - await user.type(dueDateInput, "2026-06-01T09:00"); + await user.type(dueDateInput, "2026-06-01"); await user.tab(); await waitFor(() => { - expect(updateMutate).toHaveBeenCalledWith({ - id: 1, - data: { due_at: expect.any(String) }, - }); + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { due_at: expect.any(String) } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); }); }); + + it("defaults the date to today when only a time is set", async () => { + const user = userEvent.setup(); + renderTasksPanel(); + + await user.click( + screen.getAllByRole("button", { name: "Edit task Morning routine" })[0], + ); + + const dueTimeInput = screen.getByLabelText("Due time"); + await user.type(dueTimeInput, "0900"); + await user.tab(); + + await waitFor(() => { + expect(updateMutate).toHaveBeenCalledWith( + { id: 1, data: { due_at: expect.any(String) } }, + expect.objectContaining({ onSuccess: expect.any(Function), onError: expect.any(Function) }), + ); + }); + + const lastCall = updateMutate.mock.calls.at(-1) as [{ data: { due_at: string } }, unknown]; + const committedDate = new Date(lastCall[0].data.due_at); + const today = new Date(); + expect(committedDate.getFullYear()).toBe(today.getFullYear()); + expect(committedDate.getMonth()).toBe(today.getMonth()); + expect(committedDate.getDate()).toBe(today.getDate()); + }); + }); + + describe("timestamps tooltip", () => { + it("shows Created/Modified/Completed on click of the clock button", async () => { + const user = userEvent.setup(); + renderTasksPanel(); + + await user.click( + screen.getAllByRole("button", { name: "Edit task Morning routine" })[0], + ); + + expect(screen.queryByText("Created", { selector: "div" })).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "View task timestamps" })); + + expect(screen.getByText("Created", { selector: "div" })).toBeInTheDocument(); + expect(screen.getByText("Modified", { selector: "div" })).toBeInTheDocument(); + expect(screen.getByText("Completed", { selector: "div" })).toBeInTheDocument(); + }); }); }); diff --git a/frontend/src/components/TasksPanel/TasksPanel.tsx b/frontend/src/components/TasksPanel/TasksPanel.tsx index e8d64ca0..148ea471 100644 --- a/frontend/src/components/TasksPanel/TasksPanel.tsx +++ b/frontend/src/components/TasksPanel/TasksPanel.tsx @@ -1,4 +1,4 @@ -import React from "react"; +import React, { useRef } from "react"; import classNames from "classnames"; import EntitySearchInput from "../EntitySearchInput/EntitySearchInput"; @@ -6,7 +6,7 @@ import Button from "../Button/Button"; import PlayerItemList from "../PlayerItemList/PlayerItemList"; import Tooltip from "../Tooltip/Tooltip"; import { isTaskComplete, taskSortOptions, useTasksPanel, type ItemRecord } from "./useTasksPanel"; -import { toDatetimeLocalValue, fromDatetimeLocalValue } from "../../utils/formatUtils"; +import { toDateInputValue, toTimeInputValue, fromDateAndTimeInputValues } from "../../utils/formatUtils"; import styles from "./TasksPanel.module.scss"; interface TasksPanelProps { @@ -31,9 +31,11 @@ export default function TasksPanel({ visibleTasks, getChildren, topLevelTasks, - addSubtaskParent, + pendingOpenTaskId, + hiddenItemIds, startAddSubtask, - clearAddSubtaskParent, + clearPendingOpenTaskId, + discardDraftTask, handleCreateTask, handleSubmitForm, handleEdit, @@ -48,35 +50,27 @@ export default function TasksPanel({ updateTask, } = useTasksPanel(openTaskId, onOpenNote); + // Only one task's edit summary is ever open at a time (it renders inside a modal), so a + // single pair of refs is enough to read the sibling input's value when committing due_at. + const dueDateInputRef = useRef(null); + const dueTimeInputRef = useRef(null); + if (isLoading) return

    Loading tasks...

    ; return (
    - {addSubtaskParent && ( - - Subtask of {addSubtaskParent.name} - - - )} setNewName(v)} - onCreate={(name) => handleCreateTask(name, { parent: addSubtaskParent?.id ?? undefined })} - placeholder={addSubtaskParent ? "New subtask name" : "New task name"} + onCreate={(name) => handleCreateTask(name)} + placeholder="New task name" className={styles.addTaskInput} /> @@ -107,28 +101,20 @@ export default function TasksPanel({ ); }} - renderEditSummary={(taskItem) => { + renderEditSummary={(taskItem, saveHelpers) => { + if (taskItem.id < 0) { + // An unsaved draft subtask: nothing to show or edit here yet + // (due date, parent, notes) until it's actually been created. + return
    Type a name to create this subtask.
    ; + } + const summary = getTaskEditSummary(taskItem); const hasSubtasks = (taskItem.subtask_count ?? 0) > 0; const parentOptions = topLevelTasks.filter((t) => t.id !== taskItem.id); + const parentTask = topLevelTasks.find((t) => t.id === taskItem.parent) ?? null; return ( <> -
    -
    -
    Created
    -
    {summary.created}
    -
    -
    -
    Modified
    -
    {summary.modified}
    -
    -
    -
    Completed
    -
    {summary.completed}
    -
    -
    -
    Total time: {summary.totalTime}
    @@ -154,37 +140,96 @@ export default function TasksPanel({ })() ) : null}
    -
    - {taskItem.parent == null && ( -
    - +
    + + {parentTask ? ( + + {parentTask.name} + + + ) : ( - updateTask.mutate({ - id: taskItem.id, - data: { parent: event.target.value ? Number(event.target.value) : null }, - }) - } + defaultValue="" + onChange={(event) => { + saveHelpers.reportSaving(); + updateTask.mutate( + { + id: taskItem.id, + data: { parent: event.target.value ? Number(event.target.value) : null }, + }, + { onSuccess: saveHelpers.reportSaved, onError: saveHelpers.reportError }, + ); + }} > {parentOptions.map((option) => ( @@ -212,11 +261,45 @@ export default function TasksPanel({ ))} -
    - )} + )} +
    ); }} + renderTitleRowActions={(task) => { + if (task.id < 0) return null; + const summary = getTaskEditSummary(task); + return ( + +
    +
    Created
    +
    {summary.created}
    +
    +
    +
    Modified
    +
    {summary.modified}
    +
    +
    +
    Completed
    +
    {summary.completed}
    +
    +
    + } + > + + + ); + }} hoverEdit renderRowActions={(task) => ( <> @@ -252,8 +335,13 @@ export default function TasksPanel({ )} onEdit={handleEdit} onDelete={handleDelete} - openItemId={openTaskId} - onOpenItemHandled={onOpenTaskHandled} + openItemId={openTaskId ?? pendingOpenTaskId} + onOpenItemHandled={() => { + onOpenTaskHandled?.(); + clearPendingOpenTaskId(); + }} + hiddenItemIds={hiddenItemIds} + onModalClose={discardDraftTask} sortOptions={taskSortOptions} controls={