-
Notifications
You must be signed in to change notification settings - Fork 0
feat(groups): add active group switcher #273
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
chiptus
wants to merge
21
commits into
main
Choose a base branch
from
claude/implement-124-f9vl65
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
1a3b189
feat(groups): add persistent Active Group switcher + Vote Perspective…
claude 2e2b9a0
fix(e2e): update active-group test for the new switcher's button role
claude d577ba3
refactor(groups): address PR review feedback
claude 74988bd
Fix: selecting Everyone in the switcher didn't stick for single-group…
claude b9b3c5f
Add a Manage groups link to the Active Group switcher
claude 9184370
Land the Active Group domain docs from the stale design branch
claude ffa6795
Fix Prettier formatting on the newly added domain docs
claude 6f37393
Rework Active Group into two independent settings: Active group + Act…
claude d0d35d8
Decompose ActiveGroupSwitcher.tsx into smaller files
claude bbf96bc
Drop the back-to-default pill from the Active Group switcher
claude b042ade
update migrations time
chiptus 115ee17
remove comments
chiptus 11c40b8
fix: use scope-independent activeGroupId for Vote Perspective toggle
chiptus 9954b9e
refactor: use ToggleGroup for Active group/scope settings
chiptus 3baa05c
refactor: split FilteredSetsPanel and dedupe active-scope mutations
claude 52ae67a
refactor: move group switcher files into a GroupSwitcher folder
claude 4be492c
refactor: collapse profile mutation to a single instance
claude 760db88
refactor: move ScopeMenuBody helper fns below the return
claude b1aed68
refactor: make SignInRequired's description a required prop
claude aad81db
refactor: turn settingsToggleItemClassName into a component
claude 5193d1b
refactor: squash the active_group_selected migration into active_scope
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # Active Group model for group-scoped votes | ||
|
|
||
| Status: accepted | ||
|
|
||
| We needed a way for users to view sets/artists filtered or scored by a Group's votes, on both the Schedule and Artists tabs, without re-selecting a group on every screen. We considered scoping group choice per festival edition (since a user's group might differ per festival), and considered a transient "peek at another group" mode that wouldn't overwrite the user's normal default. We rejected both: per-edition storage adds a second axis of state for a case we're not confident is common enough to justify, and a transient peek mode is a second concept (session-only vs. persisted) for uncertain benefit. Instead there is exactly one **Active Group** per user, global across editions, defaulted automatically when the user has exactly one Group and has never made an explicit choice. | ||
|
|
||
| Votes have no `group_id` (confirmed: `votes` belongs to the user alone — see `supabase/migrations/20250620065433_create_artists_table.sql`); Groups are purely a viewing/aggregation lens, never an identity a vote is recorded against. | ||
|
|
||
| ## Two independent settings, not one flat pin target | ||
|
|
||
| An earlier version of this model stored a single nullable `active_group_id` and used `NULL` to mean both "never chosen" and "explicitly Everyone" — this shipped briefly, broke "Everyone" for any single-Group user (auto-activation always won), and was caught by manual testing. Root-causing rather than patching around it again: "which group is mine" and "which lens am I viewing through" are two independent questions. | ||
|
|
||
| - **`profiles.active_group_id`** — which of the user's Groups is theirs. A mostly-static, membership-like choice. `NULL` unambiguously means "no group chosen"; auto-derives to the sole Group when the user has exactly one. | ||
| - **`profiles.active_scope`** (`group` | `everyone` | `me`, nullable) — which lens is applied by default. When `group`, it resolves through `active_group_id`. `NULL` means "never explicitly chosen" — auto-derives the same way `active_group_id` used to: the sole Group when there's exactly one, else Everyone. | ||
|
|
||
| Both are set explicitly only from **Settings** (`src/pages/Settings/`) — this is the only "make it permanent" action; there is no separate per-pick "pin" button elsewhere. | ||
|
|
||
| ## Asymmetric by design: header is a transient override, Settings holds the pin | ||
|
|
||
| The header switcher (`ActiveGroupSwitcher.tsx`) no longer writes either column directly. Selecting an entry there sets a **transient, in-memory override** (`ActiveScopeContext`, plain `useState`, not persisted) — it reverts to the Settings pin on a fresh visit/reload. | ||
|
|
||
| This is intentionally asymmetric, not symmetric: real Group picks are meant to stay sticky/default with no friction (that's the whole point of this epic — centralizing the app around "your crew"), while Everyone/Me are meant to default to being a temporary lens. Putting the friction on the Settings-level pin (not on casual switching in the header) serves that goal; a uniform "every header pick is a commit" model — the original #124 shape — does not. | ||
|
|
||
| The header dropdown lists the pinned entry first (starred), then remaining Groups, then remaining of Everyone/Me — so reverting to the pin is always the first item in the list, one open + one click. There's deliberately no separate "back to default" affordance next to the trigger: it would just add a second way to do what the starred, always-first entry already does. | ||
|
|
||
| ## Unifies with the future Schedule-tab Vote Scope | ||
|
|
||
| The three-way scope (`group` / `everyone` / `me`) is written to serve both the Artists tab's Vote Perspective (Everyone ↔ Group, no Me) and the not-yet-built Schedule tab's Vote Scope (Me ↔ Group, no Everyone — issue #125) from one seam, rather than each inventing its own resolution/auto-activation logic. Vote Perspective and Vote Scope remain independent, tab-local _toggles_ — a user's Everyone/Group choice on Artists and Me/Group choice on Schedule can still differ at once — but both read their default from the same shared `ActiveScopeContext`, and "Me" simply isn't meaningful to Vote Perspective's rating aggregation, so it's treated as Everyone there. | ||
|
|
||
| ## Considered Options | ||
|
|
||
| - **Single `active_group_id` column, "never chosen" and "explicitly Everyone" both `NULL`.** Rejected: the modeling bug this ADR fixes. | ||
| - **`active_group_id` + a boolean "has explicitly chosen" flag on the same nullable column.** Considered and briefly shipped (`active_group_selected`). Rejected in favor of the two-setting model below: it papered over the ambiguity rather than removing it, and gave "Active group" and "Active scope" no separate existence — a user could not pin "my crew" as a standing identity while defaulting their day-to-day view to Everyone. | ||
| - **Two independent settings, header as a durable pin for both (chosen for `active_group_id`, rejected as-is for the header/scope relationship).** Symmetric treatment of every switcher entry (uniform pin-on-select) was the original #124 shape. Rejected once reframed against the epic's actual goal: it puts equal friction on picking your crew and picking a one-off "everyone" peek, when the two should not have equal friction. | ||
| - **Two independent settings + asymmetric pin-in-Settings, transient header override (chosen).** Prototyped as three UI variants (`chiptus/UpLine#288`, throwaway, never merged) before picking the flat starred-dropdown shape described above. | ||
|
|
||
| ## Consequences | ||
|
|
||
| - `src/lib/activeGroup.ts` exports `resolveActiveGroupId` (unchanged shape, now scope-independent) and `resolvePinnedScope` (new), both pure and framework-free. | ||
| - `src/contexts/ActiveScopeContext.tsx` is the single seam every scope-aware surface reads from: `pinned` (durable), `current` (transient), and the two Settings-only mutators `setActiveGroup` / `setActiveScope`. | ||
| - `src/pages/EditionView/tabs/VoteTab/FilteredSetsPanel.tsx`'s Vote Perspective toggle seeds its default from `current`, mapping `me` to `everyone` since Vote Perspective has no Me option. | ||
| - Any future Vote Scope work (Schedule tab, Me ↔ Active Group, issue #125) reuses `ActiveScopeContext` directly; it does not need its own auto-activation or override tracking. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { useMutation, useQueryClient } from "@tanstack/react-query"; | ||
| import { useToast } from "@/hooks/use-toast"; | ||
| import { supabase } from "@/integrations/supabase/client"; | ||
| import { profileKeys } from "@/api/auth/types"; | ||
| import type { Database } from "@/integrations/supabase/types"; | ||
|
|
||
| type ProfileUpdate = Database["public"]["Tables"]["profiles"]["Update"]; | ||
| type ScopeColumn = "active_group_id" | "active_scope"; | ||
|
|
||
| export function useProfileFieldMutation() { | ||
| const queryClient = useQueryClient(); | ||
| const { toast } = useToast(); | ||
|
|
||
| return useMutation({ | ||
| mutationFn: async (variables: { | ||
| userId: string; | ||
| column: ScopeColumn; | ||
| value: ProfileUpdate[ScopeColumn]; | ||
| errorMessage: string; | ||
| }) => { | ||
| const { error } = await supabase | ||
| .from("profiles") | ||
| .update({ [variables.column]: variables.value } as ProfileUpdate) | ||
| .eq("id", variables.userId); | ||
|
|
||
| if (error) { | ||
| throw new Error(variables.errorMessage); | ||
| } | ||
| }, | ||
| onSuccess: (_data, variables) => { | ||
| queryClient.invalidateQueries({ | ||
| queryKey: profileKeys.detail(variables.userId), | ||
| }); | ||
| }, | ||
| onError: (error, variables) => { | ||
| toast({ | ||
| title: "Error", | ||
| description: error?.message || variables.errorMessage, | ||
| variant: "destructive", | ||
| }); | ||
| }, | ||
| }); | ||
| } |
67 changes: 67 additions & 0 deletions
67
src/components/layout/AppHeader/GroupSwitcher/ActiveGroupSwitcher.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import { ChevronDown, Settings } from "lucide-react"; | ||
| import { Link } from "@tanstack/react-router"; | ||
| import { Button } from "@/components/ui/button"; | ||
| import { | ||
| DropdownMenu, | ||
| DropdownMenuContent, | ||
| DropdownMenuItem, | ||
| DropdownMenuSeparator, | ||
| DropdownMenuTrigger, | ||
| } from "@/components/ui/dropdown-menu"; | ||
| import { useActiveScope } from "@/contexts/ActiveScopeContext"; | ||
| import { scopeIcon, scopeLabel } from "./scopeDisplay"; | ||
| import { ScopeMenuBody } from "./ScopeMenuBody"; | ||
|
|
||
| interface ActiveGroupSwitcherProps { | ||
| isMobile: boolean; | ||
| className: string; | ||
| } | ||
|
|
||
| export function ActiveGroupSwitcher({ | ||
| isMobile, | ||
| className, | ||
| }: ActiveGroupSwitcherProps) { | ||
| const { groups, pinned, current, selectScope } = useActiveScope(); | ||
|
|
||
| const CurrentIcon = scopeIcon(current); | ||
| const currentLabel = scopeLabel(current, groups); | ||
|
|
||
| return ( | ||
| <DropdownMenu> | ||
| <DropdownMenuTrigger asChild> | ||
| <Button | ||
| variant="outline" | ||
| size={isMobile ? "sm" : "default"} | ||
| className={className} | ||
| aria-label={isMobile ? `Active scope: ${currentLabel}` : undefined} | ||
| > | ||
| <CurrentIcon className="h-4 w-4" /> | ||
| {!isMobile && ( | ||
| <span className="ml-2 flex items-center gap-1"> | ||
| {currentLabel} | ||
| <ChevronDown className="h-3 w-3" /> | ||
| </span> | ||
| )} | ||
| </Button> | ||
| </DropdownMenuTrigger> | ||
| <DropdownMenuContent className="bg-gray-800 border-purple-400/30"> | ||
| <ScopeMenuBody | ||
| groups={groups} | ||
| pinned={pinned} | ||
| current={current} | ||
| onSelect={selectScope} | ||
| /> | ||
| <DropdownMenuSeparator className="bg-purple-400/30" /> | ||
| <DropdownMenuItem | ||
| asChild | ||
| className="text-purple-100 hover:bg-purple-600/30" | ||
| > | ||
| <Link to="/groups"> | ||
| <Settings className="h-4 w-4 mr-2" /> | ||
| Manage groups | ||
| </Link> | ||
| </DropdownMenuItem> | ||
| </DropdownMenuContent> | ||
| </DropdownMenu> | ||
| ); | ||
| } |
107 changes: 107 additions & 0 deletions
107
src/components/layout/AppHeader/GroupSwitcher/ScopeMenuBody.tsx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| import { Star } from "lucide-react"; | ||
| import { | ||
| DropdownMenuItem, | ||
| DropdownMenuSeparator, | ||
| } from "@/components/ui/dropdown-menu"; | ||
| import { cn } from "@/lib/utils"; | ||
| import { scopeIcon, scopeKey, scopeLabel } from "./scopeDisplay"; | ||
| import type { PinnedScope } from "@/lib/activeGroup"; | ||
| import type { Group } from "@/api/groups/types"; | ||
|
|
||
| export function ScopeMenuBody({ | ||
| groups, | ||
| pinned, | ||
| current, | ||
| onSelect, | ||
| }: { | ||
| groups: Group[]; | ||
| pinned: PinnedScope; | ||
| current: PinnedScope; | ||
| onSelect: (scope: PinnedScope) => void; | ||
| }) { | ||
| const pinnedGroupId = pinned.kind === "group" ? pinned.groupId : undefined; | ||
| const otherGroups = groups.filter((group) => group.id !== pinnedGroupId); | ||
| const otherScopeKinds = (["everyone", "me"] as const).filter( | ||
| (kind) => pinned.kind !== kind, | ||
| ); | ||
|
|
||
| return ( | ||
| <> | ||
| <ScopeMenuRow | ||
| scope={pinned} | ||
| label={scopeLabel(pinned, groups)} | ||
| isPinned={isPinned(pinned)} | ||
| isActive={isActive(pinned)} | ||
| onSelect={onSelect} | ||
| /> | ||
| <DropdownMenuSeparator className="bg-purple-400/30" /> | ||
| {otherGroups.map((group) => { | ||
| const scope: PinnedScope = { kind: "group", groupId: group.id }; | ||
| return ( | ||
| <ScopeMenuRow | ||
| key={group.id} | ||
| scope={scope} | ||
| label={group.name} | ||
| isPinned={isPinned(scope)} | ||
| isActive={isActive(scope)} | ||
| onSelect={onSelect} | ||
| /> | ||
| ); | ||
| })} | ||
| {otherGroups.length > 0 && otherScopeKinds.length > 0 && ( | ||
| <DropdownMenuSeparator className="bg-purple-400/30" /> | ||
| )} | ||
| {otherScopeKinds.map((kind) => { | ||
| const scope: PinnedScope = { kind }; | ||
| return ( | ||
| <ScopeMenuRow | ||
| key={kind} | ||
| scope={scope} | ||
| label={kind === "everyone" ? "Everyone" : "Me"} | ||
| isPinned={isPinned(scope)} | ||
| isActive={isActive(scope)} | ||
| onSelect={onSelect} | ||
| /> | ||
| ); | ||
| })} | ||
| </> | ||
| ); | ||
|
|
||
| function isPinned(scope: PinnedScope) { | ||
| return scopeKey(scope) === scopeKey(pinned); | ||
| } | ||
| function isActive(scope: PinnedScope) { | ||
| return scopeKey(scope) === scopeKey(current); | ||
| } | ||
| } | ||
|
|
||
| function ScopeMenuRow({ | ||
| scope, | ||
| label, | ||
| isPinned, | ||
| isActive, | ||
| onSelect, | ||
| }: { | ||
| scope: PinnedScope; | ||
| label: string; | ||
| isPinned: boolean; | ||
| isActive: boolean; | ||
| onSelect: (scope: PinnedScope) => void; | ||
| }) { | ||
| const Icon = scopeIcon(scope); | ||
| return ( | ||
| <DropdownMenuItem | ||
| onClick={() => onSelect(scope)} | ||
| className={cn( | ||
| "flex items-center gap-2 text-purple-100 hover:bg-purple-600/30", | ||
| isActive && "bg-purple-600/20", | ||
| )} | ||
| > | ||
| <Icon className="h-4 w-4" /> | ||
| <span className="flex-1">{label}</span> | ||
| {isPinned && ( | ||
| <Star className="h-3.5 w-3.5 fill-amber-400 text-amber-400" /> | ||
| )} | ||
| </DropdownMenuItem> | ||
| ); | ||
| } | ||
27 changes: 27 additions & 0 deletions
27
src/components/layout/AppHeader/GroupSwitcher/scopeDisplay.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import { Globe, User as UserIcon, Users } from "lucide-react"; | ||
| import type { PinnedScope } from "@/lib/activeGroup"; | ||
| import type { Group } from "@/api/groups/types"; | ||
|
|
||
| export function scopeKey(scope: PinnedScope): string { | ||
| return scope.kind === "group" ? `group:${scope.groupId}` : scope.kind; | ||
| } | ||
|
|
||
| export function scopeLabel(scope: PinnedScope, groups: Group[]): string { | ||
| if (scope.kind === "everyone") { | ||
| return "Everyone"; | ||
| } | ||
| if (scope.kind === "me") { | ||
| return "Me"; | ||
| } | ||
| return groups.find((group) => group.id === scope.groupId)?.name ?? "Group"; | ||
| } | ||
|
|
||
| export function scopeIcon(scope: PinnedScope) { | ||
| if (scope.kind === "everyone") { | ||
| return Globe; | ||
| } | ||
| if (scope.kind === "me") { | ||
| return UserIcon; | ||
| } | ||
| return Users; | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.