Skip to content

Conversation

@Mati0x
Copy link
Collaborator

@Mati0x Mati0x commented Dec 5, 2025

Filter and Sorting UX/UI. -> Clean and quick status filters with sort by 5 options aside. Active status and sort by most conviction as defaults.

When there are NO proposal yet in pool, this component is not shown.

Also I group: download proposals conviction button with add create proposal next to a ". . . " icon to keep UI cleaner

…ol page

- cleaned up code and tested on Arbitrum Sepolia + 1Hive community
- upgrade layout and modal open/close logic
- apply small tweaks to ProposalCard
@vercel
Copy link

vercel bot commented Dec 5, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
gardens-v2 Ready Ready Preview 💬 6 unresolved Dec 9, 2025 2:31am

@Mati0x Mati0x changed the title Filter sorting proposals Filter & sorting proposals in pool page Dec 8, 2025
Copy link
Contributor

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 implements a filter and sorting UI/UX for proposals in the pool page. It introduces status filters (all, active, disputed, executed, cancelled) and sort options (newest, oldest, most supported, most requested, most conviction), with active status and most conviction as defaults. The UI is reorganized to show filters/sorting only when proposals exist, and the "Manage Support" allocation view is moved to a modal.

Key Changes

  • New ProposalsModalSupport component for handling proposal support allocation in a modal interface
  • useProposalFilter custom hook for managing filter and sort state with loading transitions
  • Refactored layout with filters/sorting UI component and repositioned action buttons
  • PoolGovernance and related components moved to the right sidebar alongside PoolMetrics

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 18 comments.

Show a summary per file
File Description
ProposalsModalSupport.tsx New component for managing proposal support in a modal view
Proposals.tsx Major refactor: adds filtering/sorting UI, moves allocation view to modal, introduces useProposalFilter hook
ProposalCard.tsx Simplified to remove allocation view logic, now only displays proposal information
PoolMetrics.tsx Layout adjustments to remove wrapper div and simplify z-index handling
PoolHeader.tsx Layout width adjustments for consistent grid sizing
PoolGovernance.tsx Added wrapper div with z-index adjustment for proper layering
client-page.tsx Restructured layout to include PoolGovernance in sidebar, added "Have an idea?" section

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

return {
filter,
setFilter: setFilterWithLoading,
sortBy: sortBy,
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The return value sortBy: sortBy is redundant. Consider simplifying to just sortBy in the object shorthand.

Suggested change
sortBy: sortBy,
sortBy,

Copilot uses AI. Check for mistakes.
<div className="flex gap-2 sm:justify-between flex-wrap ">
{FILTERS.map((f) => (
<Button
onClick={() => setFilter(f === filter ? null : f)}
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The onClick handler toggles the filter to null when the same filter is clicked again, but the logic filter === f ? null : f means clicking "all" when "all" is already selected would set the filter to null. This could lead to unexpected behavior. Consider whether filter should never be null or if "all" should remain selected when clicked again.

Suggested change
onClick={() => setFilter(f === filter ? null : f)}
onClick={() => { if (f !== filter) setFilter(f); }}

Copilot uses AI. Check for mistakes.
</>
)}

<div className="section-layout flex flex-col items-start xl:items-center gap-4 -z-50">
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The class -z-50 is used here as well. Applying such a low z-index to a section-layout component could cause visibility issues. Verify this is intentional and document why it's needed, or consider a more targeted approach.

Suggested change
<div className="section-layout flex flex-col items-start xl:items-center gap-4 -z-50">
<div className="section-layout flex flex-col items-start xl:items-center gap-4">

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@Corantin Corantin Dec 9, 2025

Choose a reason for hiding this comment

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

Here is why these sections are not interactable nnow


const chainId = useChainIdFromPath();

//New querys and logic for PoolGovernance component
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

Comment "New querys and logic for PoolGovernance component" contains a typo: "querys" should be "queries".

Suggested change
//New querys and logic for PoolGovernance component
//New queries and logic for PoolGovernance component

Copilot uses AI. Check for mistakes.
Comment on lines +991 to +996
const filteredProposals = useMemo(() => {
if (!filter || filter == "all") return proposals;

const status = FILTER_STATUS[filter];
return proposals.filter((p) => Number(p.proposalStatus) === status);
}, [filter, proposals]);
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

The useMemo dependency array is missing FILTER_STATUS. If FILTER_STATUS changes between renders (though it's defined as a constant in the function), it won't trigger a recomputation. Consider moving FILTER_STATUS outside the hook or including it in the dependency array.

Copilot uses AI. Check for mistakes.
/* eslint-disable jsx-a11y/click-events-have-key-events */
"use client";

import { forwardRef, useEffect, useImperativeHandle } from "react";
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

Unused import useEffect.

Suggested change
import { forwardRef, useEffect, useImperativeHandle } from "react";
import { forwardRef, useImperativeHandle } from "react";

Copilot uses AI. Check for mistakes.
thresholdPct,
totalSupportPct,
timeToPass,
triggerConvictionRefetch,
Copy link

Copilot AI Dec 9, 2025

Choose a reason for hiding this comment

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

Unused variable triggerConvictionRefetch.

Suggested change
triggerConvictionRefetch,

Copilot uses AI. Check for mistakes.
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