Skip to content

Replace run type single-select with multi-select checkbox dropdown#31

Merged
aleveau merged 1 commit into
mainfrom
feat/run-type-multiselect
May 10, 2026
Merged

Replace run type single-select with multi-select checkbox dropdown#31
aleveau merged 1 commit into
mainfrom
feat/run-type-multiselect

Conversation

@aleveau
Copy link
Copy Markdown
Collaborator

@aleveau aleveau commented May 10, 2026

Summary

  • Replaces the run type <select> in FilterBar with a multi-select checkbox dropdown
  • Users can now filter charts by multiple run types simultaneously (e.g. Easy + Long)
  • Empty selection = show all types (preserves existing default behaviour)
  • Click-outside closes the dropdown, matching the split-type filter UX pattern

Test plan

  • Open the charts/activities page
  • Confirm the run type filter shows "All run types" by default
  • Check "Easy" → only easy runs appear in charts and table
  • Check both "Easy" and "Long" → both run types appear
  • Uncheck all → all runs appear again
  • Click "Clear filters" → run type selection resets

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request converts the 'Run type' filter from a single-select dropdown to a multi-select component using a Set to manage selected types. Feedback focuses on improving the user experience by sorting and truncating the selection labels, enhancing accessibility with ARIA attributes and explicit button types, and mitigating risks associated with using mutable Set objects in shared default constants.

Comment on lines +52 to +55
const runTypeLabel =
filters.runTypes.size === 0
? "All run types"
: [...filters.runTypes].map((t) => RUN_TYPE_LABELS[t] ?? t).join(", ");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The current label logic produces unsorted strings based on selection order and can become excessively long if many types are selected. Additionally, it doesn't handle the case where all types are selected, which is functionally equivalent to selecting none. Consider sorting the labels and truncating the display for better UX.

  const selectedRunTypes = ALL_RUN_TYPES.filter((t) => filters.runTypes.has(t));
  const runTypeLabel =
    selectedRunTypes.length === 0 || selectedRunTypes.length === ALL_RUN_TYPES.length
      ? "All run types"
      : selectedRunTypes.length > 2
      ? `${selectedRunTypes.length} types selected`
      : selectedRunTypes.map((t) => RUN_TYPE_LABELS[t] ?? t).join(", ");

Comment on lines +111 to +119
<button
onClick={() => setShowRunTypeFilter((v) => !v)}
className="flex items-center gap-1.5 text-sm border border-gray-200 rounded-md px-2.5 py-1.5 bg-gray-50 focus:outline-none focus:ring-2 focus:ring-blue-500 text-gray-700"
>
<span>{runTypeLabel}</span>
<svg className="w-3.5 h-3.5 text-gray-400" fill="none" viewBox="0 0 24 24" stroke="currentColor">
<path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M19 9l-7 7-7-7" />
</svg>
</button>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The custom dropdown button should include ARIA attributes for accessibility and explicitly set type="button" to prevent accidental form submissions. It's also a good practice to flip the chevron icon when the dropdown is open to provide visual feedback.

          <button
            type="button"
            aria-haspopup="listbox"
            aria-expanded={showRunTypeFilter}
            onClick={() => setShowRunTypeFilter((v) => !v)}
            className="flex items-center gap-1.5 text-sm border border-gray-200 rounded-md px-2.5 py-1.5 bg-gray-50 focus:outline-none focus:ring-2 focus:ring-blue-500 text-gray-700"
          >
            <span>{runTypeLabel}</span>
            <svg className="w-3.5 h-3.5 text-gray-400" fill="none" viewBox="0 0 24 24" stroke="currentColor">
              <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d={showRunTypeFilter ? "M5 15l7-7 7 7" : "M19 9l-7 7-7-7"} />
            </svg>
          </button>

distanceRange: "all",
nameSearch: "",
runType: "all",
runTypes: new Set(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Using new Set() inside a shared constant like DEFAULT_FILTERS is risky because Set is a mutable object in JavaScript. If any part of the application accidentally mutates the filter state directly (e.g., filters.runTypes.add(...)), it will permanently alter the default values for the entire session. It is safer to use a factory function to generate default filters or use a primitive array for the state.

@aleveau aleveau merged commit f755d1d into main May 10, 2026
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.

1 participant