Skip to content

Conversation

xeniag
Copy link
Member

@xeniag xeniag commented Nov 10, 2026 •

  • Updated cache logic to replace the current search page instead of merging results.
  • Introduced searchKey and pageKey functions for better cache key management.
  • Removed redundant SEARCH_CATALOG_MORE_QUERY and adjusted related logic in ElementsPage.
  • Implemented pagination controls in ElementsPage for navigating between pages.
  • Added utility functions for managing scroll positions during result loading.
  • Enhanced tests to cover new pagination behavior and ensure correct cache interactions.

…rsor handling

- Updated cache logic to replace the current search page instead of merging results.
- Introduced `searchKey` and `pageKey` functions for better cache key management.
- Removed redundant `SEARCH_CATALOG_MORE_QUERY` and adjusted related logic in `ElementsPage`.
- Implemented pagination controls in `ElementsPage` for navigating between pages.
- Added utility functions for managing scroll positions during result loading.
- Enhanced tests to cover new pagination behavior and ensure correct cache interactions.
@xeniag xeniag changed the title Refactor Apollo cache to manage search results with pagination and cu… feat: implement pagination on Elements page Nov 10, 2026
@xeniag xeniag marked this pull request as ready for review November 10, 2026 19:14
@xeniag xeniag requested a review from a team as a code owner November 10, 2026 19:14
Copy link
Member

@ronjalh ronjalh left a comment •

Choose a reason for hiding this comment

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

Claude review:

  • Resultatlisten forsvinner mens neste side lastes. Med 700 ms treg API ble listen byttet ut med «Loading elements…», og pagineringen forsvant. På dev ble de gamle radene stående (previousData). Det er en synlig UX-forverring og gir layout-hopp. Dette er det eneste jeg ville rettet før merge.
  • Forvirrende tekst under listen. Den sier «Page 29, showing 2 results», «Showing 2 of 562» og «Page 29» samtidig. «Showing 2 of 562» betydde før «alle hittil», men betyr nå bare denne siden. Det er misvisende og duplisert.
  • hvorfor telles totalcount på hver side? er det ikke alltid maks med unntak av hvis det er siste side? totalCount telles på hver side. Det er en bevisst avgjørelse som er dokumentert i docs/API.md: kostnaden er cappet, ca. 30–100 ms. Men det går på bekostning av bærekraft-poenget i issuet.
  • Hvorfor har du disablet eslint for react hooks? fra pkt: Små ting. cache.gc() kjører etter hver datalasting, og det er en eslint-disable for react-hooks..
  • Uenig i valg av url, den er litt vel lang. URL-en vokser per side. Hver cursor er ca. 100 tegn. På side 29 var adressen 3067 tegn. Det er dokumentert og greit under nettlesergrensene, men vær klar over det. Claude foreslår: Jeg anbefaler å lagre stakken i location.state. Det er en liten endring i ElementsPage.tsx, og URL-en slutter å vokse. Prisen er at en delt lenke til side 5 mister Previous-historikken. Det som gjenstår som ulempe, er at det blir to kilder til tilstand (URL og state) som må holdes i synk. Det må testes, blant annet reload, Tilbake/Frem og en lenke uten state.

Base automatically changed from dev to main November 10, 2026 19:45
@ronjalh ronjalh changed the base branch from main to dev November 10, 2026 19:48
- Update pagination logic to use cursor-based navigation with `after` parameter instead of `cursor`.
- Modify displayed result text to show ranges (e.g., "Showing 1–20 of 562") instead of total counts.
- Adjust GraphQL queries to conditionally request total counts based on pagination state.
- Enhance test cases to validate new pagination behavior and result display.
- Ensure that the page state is preserved across navigations and reloads, improving user experience.
@ronjalh
Copy link
Member

ronjalh commented Nov 11, 2026

Anbefaler å legge inn at den går til første side ved feil i lenken (eks utdatert cursor) for brukeropplevelsen. nå går den vel til try again eller en feil.

fokus etter feil er noe vi burde tenke på. pageChangePendingFocus blir stående true hvis hentingen feiler. Fokus hopper da til «Results» ved en senere, urelatert lasting.

du har fortsatt eslint disable på en react hook, men kommer ikke til å blokkere merge på det.

så resolve conflicts og så burde det være good to merge.

Copy link
Member

@ronjalh ronjalh left a comment

Choose a reason for hiding this comment

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

Fiks det jeg kommenterte på, så kan du merge den direkte selv :). Får ikke sett på det på en stund så da er nok dette beste løsning.

Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants