Skip to content

Feature: Multi-select on relevant pages#148

Merged
jacc merged 12 commits into
mainfrom
multi-select
Jun 21, 2025
Merged

Feature: Multi-select on relevant pages#148
jacc merged 12 commits into
mainfrom
multi-select

Conversation

@jacc
Copy link
Copy Markdown
Collaborator

@jacc jacc commented Jun 16, 2025

CleanShot.2025-06-15.at.22.06.56.mp4

@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
stardew ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 21, 2025 10:26pm

@jacc jacc requested review from clxmente and Copilot June 16, 2025 05:07
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a multi-select feature and replaces individual filter buttons with a ToggleGroup UI on several pages to streamline filtering and bulk actions.

  • Introduce MultiSelectProvider context and bulk action dialogs across pages (shipping, museum, walnuts, fishing, crafting, cooking).
  • Replace FilterButton components with ToggleGroup for single-select filters and rearrange filter/search layouts.
  • Implement shift-click range selection in recipe cards and add bulk action buttons to sheets.

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/pages/shipping.tsx Swapped filter buttons for ToggleGroup UI
src/pages/relationships.tsx Added ToggleGroup for relationships filter
src/pages/museum.tsx Integrated multi-select and bulk actions
src/pages/island/walnuts.tsx Added multi-select & bulk actions
src/pages/fishing.tsx Added multi-select & bulk actions
src/pages/crafting.tsx Shift+click selection in recipe cards
src/pages/cooking.tsx Shift+click selection & bulk actions
src/pages/_app.tsx Wrapped app in MultiSelectProvider
src/contexts/multi-select-context.tsx New multi-select context definition
src/components/dialogs/bulk-action-dialog.tsx BulkActionDialog implementation
src/components/cards/recipe-card.tsx Multi-select in recipe cards
Comments suppressed due to low confidence (4)

src/pages/island/walnuts.tsx:288

  • Using type="museum" for the walnuts page’s bulk-action dialog is semantically misleading. Consider extending BulkActionDialog to support a dedicated walnuts type or allow explicit label props instead of reusing the museum type.
<BulkActionDialog open={bulkActionOpen} setOpen={setBulkActionOpen} type="museum" onBulkAction={handleWalnutBulkAction} />

src/pages/shipping.tsx:70

  • [nitpick] The main filter state is named filter but the season filter still uses _seasonFilter. Rename _seasonFilter (and its setter) to seasonFilter for naming consistency and readability.
const [_seasonFilter, setSeasonFilter] = useState("all");

src/pages/fishing.tsx:387

  • [nitpick] Reusing type="shipping" for fishing bulk actions can be confusing. To better express domain intent, consider adding a fishing type to BulkActionDialog or allow an explicit label prop rather than overloading the shipping type.
<BulkActionDialog open={bulkActionOpen} setOpen={setBulkActionOpen} type="shipping" onBulkAction={handleFishingBulkAction} />

src/contexts/multi-select-context.tsx:1

  • The new multi-select context implements toggle/range logic but currently has no unit tests. Adding tests for toggleItem, addItems, removeItems, and shift-key range selection will help ensure correct behavior and prevent regressions.
import { createContext, useContext, useState, ReactNode } from "react";

onClick={() => handleBulkAction(2)}
disabled={selectedItems.size === 0}
>
{onBulkAction && window.location.pathname.includes("walnuts")
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Relying on window.location.pathname to switch between “Found” and “Donated” labels couples this component to the router and makes it brittle. Instead, drive the label logic via props or the type parameter.

Copilot uses AI. Check for mistakes.

const isSelected = selectedItems.has(recipe.itemID.toString());

const handleClick = (e: React.MouseEvent) => {
Copy link

Copilot AI Jun 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The lastSelectedIndex ref isn’t reset when toggling multi-select mode, which could lead to unexpected range selections. Consider clearing lastSelectedIndex.current when exiting or entering multi-select mode.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Member

@clxmente clxmente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@jacc jacc merged commit 75fc777 into main Jun 21, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants