Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create pagination on home page proposals list #2655

Merged
merged 5 commits into from
Jan 14, 2025
Merged

Create pagination on home page proposals list #2655

merged 5 commits into from
Jan 14, 2025

Conversation

adamgall
Copy link
Member

@adamgall adamgall commented Jan 8, 2025

Closes https://linear.app/decent-labs/issue/ENG-108/create-pagination-controls-for-proposals-list

I wanted it to exist, so I told the AI to do it, and it did.

Will create PM artifacts in the AM.

Screenshot 2025-01-08 at 12 11 03 AM

Copy link

netlify bot commented Jan 8, 2025

Deploy Preview for decent-interface-dev ready!

Name Link
🔨 Latest commit 7095513
🔍 Latest deploy log https://app.netlify.com/sites/decent-interface-dev/deploys/67817b5d1beb8a0008442055
😎 Deploy Preview https://deploy-preview-2655.app.dev.decentdao.org
📱 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 site configuration.

@DarksightKellar
Copy link
Contributor

Holy hell, what? This has gotta be the best ad for Cursor yet.

@adamgall
Copy link
Member Author

adamgall commented Jan 8, 2025

Cursor loves working in React codebases.

Copy link
Contributor

@Da-Colon Da-Colon left a comment

Choose a reason for hiding this comment

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

Code-wise, Looks good. I want to QA this a bit. before approval. will do some testing later.

Copy link
Contributor

@mudrila mudrila left a comment

Choose a reason for hiding this comment

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

Have some comments about components usage and initial state for useState

src/components/Proposals/ProposalsList.tsx Show resolved Hide resolved
icon: ComponentType;
}

function NavButton({ onClick, isDisabled, icon: IconComponent }: NavButtonProps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why just not naming it IconComponent right on the interface?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

src/hooks/utils/usePagination.ts Outdated Show resolved Hide resolved
src/hooks/utils/usePagination.ts Show resolved Hide resolved
Copy link
Contributor

@DarksightKellar DarksightKellar left a comment

Choose a reason for hiding this comment

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

One change request, a comment about potential error handling, then should be good to go for me


const [pageSize, setPageSize] = useState(() => {
const size = searchParams.get(QUERY_PARAMS.SIZE);
return size && PAGE_SIZE_OPTIONS.includes(parseInt(size)) ? parseInt(size) : DEFAULT_PAGE_SIZE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Might as well parseInt on searchParams.get(QUERY_PARAMS.SIZE) instead. Also worried about QUERY_PARAMS having non-numeric values

Copy link
Member Author

Choose a reason for hiding this comment

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

I've cleaned up the query param validation and making sure weird user input (via query params in url) are handled properly. Please try to break it yourself!

Copy link

cloudflare-workers-and-pages bot commented Jan 9, 2025

Deploying decent-interface with  Cloudflare Pages  Cloudflare Pages

Latest commit: 7095513
Status:🚫  Build failed.

View logs

Copy link

linear bot commented Jan 10, 2025

@adamgall
Copy link
Member Author

Bug I noticed while David was screensharing and testing.

When a list of proposals is being loaded for the first time, the "last item" on the first page will get stuck on a "loading" component.

@adamgall adamgall merged commit f370b9e into develop Jan 14, 2025
7 of 8 checks passed
@adamgall adamgall deleted the pagination branch January 14, 2025 08:11
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.

5 participants