Skip to content

feat: Add component sorting by any metric in metrics.tsv - #125

Merged
eurunuela merged 3 commits into
ME-ICA:masterfrom
eurunuela:feat/sort-by-metric-124
Apr 24, 2026
Merged

feat: Add component sorting by any metric in metrics.tsv#125
eurunuela merged 3 commits into
ME-ICA:masterfrom
eurunuela:feat/sort-by-metric-124

Conversation

@eurunuela

@eurunuela eurunuela commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Summary

Closes #124

This PR adds the ability to sort components by any metric in metrics.tsv and loop over component figures in that order.

  • Clickable column headers in the Component Metrics table — click a header to sort by that metric descending, click again for ascending, click a third time to clear (back to default order)
  • Active sort column is highlighted in blue with a ↑/↓ direction indicator
  • Arrow-key navigation (←/→) loops through component figures following the same sorted order
  • Sort preference (column + direction) persists across page reloads via localStorage
  • Pie chart visual order is unchanged (it remains grouped by classification)

Changed files

  • src/Plots/ComponentTable.js — clickable <th> headers with sort indicator; row rendering now iterates sortedIndices so rows reorder while selectedIndex stays anchored to original TSV positions
  • src/Plots/Plots.js — sortColumn/sortDirection state, navigationOrder memo (drives both table and keyboard nav), handleSort callback, new props passed to ComponentTable

Test plan

  • Load a tedana report, click the Kappa header → rows sort by kappa descending, ↓ appears
  • Click Kappa again → sort switches to ascending, ↑ appears
  • Click Kappa a third time → sort clears, original order restored
  • While sorted, press ←/→ arrow keys → figure navigation follows the sort order
  • Click a different column header → sort switches to that column
  • Reload page → sort preference persists
  • Clicking any table row still selects that component in all other views (scatter, pie, time series)
  • Pie chart visual order is unaffected by sort changes

🤖 Generated with Claude Code

@eurunuela @claude
feat: Add component sorting by any metric via clickable table headers (…
747d35c 
…ME-ICA#124)

- Clicking a column header sorts the component table by that metric (desc → asc → clear)
- Active sort column is highlighted in blue with ↑/↓ indicator
- Arrow-key figure navigation follows the same sorted order
- Sort preference persists across page reloads via localStorage

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@netlify

netlify Bot commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown

✅ Deploy Preview for rica-fmri ready!

Name Link
🔨 Latest commit 5a701bd
🔍 Latest deploy log https://app.netlify.com/projects/rica-fmri/deploys/69ea7f286ceb130008b8558a
😎 Deploy Preview https://deploy-preview-125--rica-fmri.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@eurunuela

eurunuela commented Apr 23, 2026

Copy link
Copy Markdown
Collaborator Author

How is this @tsalo ?

@tsalo

tsalo commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

This looks really great! Just a couple of notes:

  1. The metrics table doesn't include all of the metrics.
  2. When I use arrow keys to loop over components in the metric table after sorting by my target metric, I can't stay scrolled up on the component plots. It keeps jumping down to the metric table when it reaches the top or bottom of the table's widget.
  3. My tedana output folder has two runs' derivatives, but it looks like only one was loaded. Support for multiple runs in the same folder seems to be mixed across RICA. The Carpets page lets me select a file so I can see all of the carpets from both runs and the Histograms tab on the QC page also lets me see both runs' figures. The rest of the pages/tabs don't seem to include both runs' data.
    • I suppose this one is a separate issue.

@eurunuela @claude
fix: Show all metrics in table and prevent scroll-jump on arrow key n…
8c98334 
…avigation

- Replace DISPLAY_COLUMNS whitelist with PRIORITY_COLUMNS that includes all
  labeled metrics (normalized variance explained, countsigFT2, countsigFS0,
  signal-noise_p, optimal sign, variance explained rank, rationale); any
  additional TSV columns are appended after the priority list
- Replace row.scrollIntoView() with container.scrollTo() so arrow-key
  navigation only scrolls within the metrics table, not the whole page

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@eurunuela eurunuela mentioned this pull request Apr 23, 2026
Open
@eurunuela

eurunuela commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@tsalo your first two points should be addressed now.

I opened #126 for the third point.

@tsalo

tsalo commented Apr 23, 2026

Copy link
Copy Markdown
Member

The new version looks great! Thank you!

@eurunuela
eurunuela marked this pull request as ready for review April 23, 2026 19:54
Copilot AI review requested due to automatic review settings April 23, 2026 19:54
Copilot AI reviewed Apr 23, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 sortable component metrics and aligns keyboard left/right navigation with the same ordering, persisting the chosen sort (column + direction) in localStorage.

Changes:

  • Added sortColumn / sortDirection state in Plots, persisted via localStorage, and used to compute a navigationOrder + sortedIndices.
  • Updated keyboard left/right navigation to follow navigationOrder instead of the pie chart’s order only.
  • Updated ComponentTable to render columns dynamically (priority list + TSV extras) and make headers clickable with a sort indicator.

Reviewed changes

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

File Description
src/Plots/Plots.js Adds sort state/persistence, computes sorted navigation order, and wires sort + indices into the table and hotkeys.
src/Plots/ComponentTable.js Adds clickable headers with sort indicator, supports extra TSV columns, and renders rows using provided sorted indices.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/Plots/Plots.js
Comment thread src/Plots/Plots.js
Comment thread src/Plots/ComponentTable.js Outdated
Comment thread src/Plots/ComponentTable.js Outdated
@eurunuela @claude
fix: Address Copilot review comments on sorting feature
5a701bd 
- Fix string-column sort: use localeCompare instead of subtraction to
  avoid NaN when sorting Component, classification, tags, etc.
- Fix null handling: push missing values to end instead of treating as 0
- Fix cleared-sort table order: sortedIndices now returns original TSV
  order when no sort column is active (keyboard arrow navigation still
  uses pieData grouping as before)
- Fix sortedIndices empty-array falsy bug: guard with Array.isArray +
  length check before using sortedIndices in table body
- Fix accessibility: move sort onClick from <th> to a <button> inside
  each header; add aria-sort attribute to active column

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@eurunuela
eurunuela merged commit a4fb707 into ME-ICA:master Apr 24, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Add option/tab to sort components by any metric in metrics.tsv

3 participants

@eurunuela @tsalo