fix(app): make kanban and gallery record expansion update the url - #3627
Open
nimser wants to merge 2 commits into
Open
fix(app): make kanban and gallery record expansion update the url#3627nimser wants to merge 2 commits into
nimser wants to merge 2 commits into
Conversation
Cards opened their record by setting local state only, so the url kept pointing at the view and "copy record url" returned a link without recordId. Expanding now goes through the router, like the grid view.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3626
Problem
In the kanban and gallery views, clicking a card expands the record by setting local component state only. The URL is never updated, so the expanded record has no
recordIdquery param and the header's Copy record URL button (which copieswindow.location.href) returns the bare view URL:https://host/base/bseXXX/table/tblXXX/viwXXXThe same record opened from the grid view gives the expected link, because grid expansion pushes
recordIdinto the router:https://host/base/bseXXX/table/tblXXX/viwXXX?recordId=recXXXFix
Expand through the router in kanban and gallery, like the grid view does:
useExpandRecordhook pushing{ ...router.query, recordId }(shallow)KanbanCard/ galleryCarduse it on click and after record insertionsetExpandRecordIdremoved from the kanban and gallery contexts: once the cards expand through the router it has no consumer left, and leaving a state setter in a shared context invites the same bug again. The providers keep their local state, synced fromrouter.query.recordId. Happy to keep the contexts untouched if you prefer a strictly minimal diffClosing the expanded record already resets the query in both providers, so no change was needed there.
Notes
recordIdfrom the router at all, so fixing it needs a larger change; left out to keep this PR focused.Tests
useExpandRecord.spec.ts(vitest,renderHook+ mockednext/router, same pattern asuseImageModelUiState.spec.tsandDataDbBadge.spec.tsx) asserts the shallow push carryingrecordId. The playwright suite underapps/nextjs-app/e2eonly covers static pages and is not wired into CI, so no e2e was added.Verification
pnpm typecheckinapps/nextjs-app: passvitest run src/features/app/blocks/view/hooks/useExpandRecord.spec.ts: passeslinton the touched directories: cleanpnpm dev): before the fix, clicking a kanban card expands the record and the URL stays.../viwXXX; after the fix it becomes.../viwXXX?recordId=recXXX, closing the record resets it, and the gallery view behaves the same