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

feat: <ItemsList /> with cursor-based pagination #445

Merged
merged 18 commits into from
Aug 29, 2023
Merged

feat: <ItemsList /> with cursor-based pagination #445

merged 18 commits into from
Aug 29, 2023

Conversation

iuioiua
Copy link
Contributor

@iuioiua iuioiua commented Aug 22, 2023

This change contains a few changes that affect interdependent features. Changes include:

  • Add <ItemsList /> with cursor-based pagination
  • Remove now unused code for the previous pagination solution
  • Fix and rework voting functionality relating to Vote count not working as expected #427
  • Tweak REST API endpoints return signature to be more generic, using a values field
  • fetchValues()
  • Simplify and optimise <VoteButton />
  • New DELETE/GET /api/items/[id]/vote endpoint
  • New GET /api/me/votes endpoint
  • Other various cleanups

Note: The migration script has been tested successfully locally.

Closes #427
Closes #414
Towards #439

islands/ItemsList.tsx Outdated Show resolved Hide resolved
@iuioiua iuioiua changed the title refactor: rework vote functionality feat: <ItemsList /> Aug 27, 2023
@iuioiua iuioiua changed the title feat: <ItemsList /> feat: <ItemsList /> with cursor-based pagination Aug 27, 2023
@iuioiua iuioiua marked this pull request as ready for review August 27, 2023 22:20
@iuioiua iuioiua mentioned this pull request Aug 29, 2023
Comment on lines +38 to +41
const itemsSig = useSignal<Item[]>([]);
const votedItemsIdsSig = useSignal<string[]>([]);
const cursorSig = useSignal("");
const isLoadingSig = useSignal(false);
Copy link
Member

Choose a reason for hiding this comment

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

I'm curious why useSignal is used here instead of useState

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM

@iuioiua iuioiua merged commit 853e6e6 into main Aug 29, 2023
@iuioiua iuioiua deleted the vote-rework branch August 29, 2023 07:19
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.

Vote count not working as expected proposal: cursor-based pagination
2 participants