Skip to content

Fix/leaderboard active filter#117

Open
Luchistack wants to merge 3 commits into
Grainlify:mainfrom
Luchistack:fix/leaderboard-active-filter
Open

Fix/leaderboard active filter#117
Luchistack wants to merge 3 commits into
Grainlify:mainfrom
Luchistack:fix/leaderboard-active-filter

Conversation

@Luchistack

Copy link
Copy Markdown
Contributor

#closes #85
Description

Fixed an issue where the activeFilter state in LeaderboardPage was not being applied to the underlying table components, causing unfiltered data to always render.
Key Changes

Implemented useMemo filtering logic in LeaderboardPage to derive filteredData.

Added conditional rendering to display a precise empty state when no rows match the filter.

Added comprehensive tests in LeaderboardPage.test.tsx covering default, filtered, and empty states.

Testing

Verified "All" selection displays the full dataset.

Verified dimension-specific filters correctly narrow results.

Verified empty state message renders when no data matches.

Security Notes

None; client-side filtering only.

#closes

@Jagadeeshftw

Copy link
Copy Markdown
Contributor

thanks for this, and the datepicker improvements here are genuinely nice (the labelled, themed, error-aware version is exactly what AdminPage needs). the blocker is the LeaderboardPage change: this branch replaces the full LeaderboardPage with a 36-line stub that takes a data prop and filters on a dimension field, but main now has the real ~395-line page that fetches its own data, has pagination, ecosystems, etc, and App.tsx renders it as <LeaderboardPage /> with no props. merging this as-is would wipe out that page and break the route. could you rebase onto the latest main and re-apply just the active-filter behaviour on top of the current LeaderboardPage (keeping the no-props, self-fetching shape)? the datepicker part can come along with the rebase. holding off on merging until then so we dont lose the existing leaderboard. appreciate it.

@Jagadeeshftw

Copy link
Copy Markdown
Contributor

thanks for this, and the active-filter idea plus the empty-state are genuinely useful. i can't merge it as-is yet though, so i'm holding it for now.

the blocker: this version replaces the full LeaderboardPage (the ~395-line component that fetches from getLeaderboard, renders the contributors podium, the tables, skeleton loaders and infinite scroll) with a ~36-line component whose signature is LeaderboardPage({ data }: { data: any[] }). the router still mounts it as <LeaderboardPage /> with no props (App.tsx isn't changed in this pr), so at runtime data is undefined and data.filter(...)/data.length throw, which would wipe the real leaderboard.

to move forward, could you rebase onto main and re-apply just the active-filter + empty-state behavior on top of the existing LeaderboardPage, keeping it self-contained (no required data prop) so it still fetches and renders the podium/tables/skeletons? if you'd rather have the page own the filter while a child does the filtering, that works too as long as the page keeps fetching its own data and the router contract stays <LeaderboardPage />. happy to re-review as soon as it's rebased. holding until then.

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.

Wire up the unused activeFilter state in LeaderboardPage to actually filter table results

2 participants