Add filtered detail navigation - #12585
Conversation
✅ Deploy Preview for inventree-web-pui-preview ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Unit TestsThis will require regression testing via playwright. |
|
@Jr-kenny this is starting to come together nicely I think that the placement is a bit wonky here - perhaps it makes more sense in the "breadcrumb" bar - and please adjust the layout to vertically center the items, larger icons, etc
|
|
An implementation request - putting all of the attributes into the URL gets pretty messy. Perhaps we can instead use a hash of the values, and then store the actual data in sessionStorage - keyed to the hash? So this way all we add is |
|
@SchrodingersGat that makes it not shareable, invalidating most of the usefulness of links |
|
Ok, I was not thinking about sharing, for me I think the main value is the quick navigation within the user's own table context. |
|
Yeah, I see what you mean. The current implementation puts the API URL, filters, index, PK, and field directly in the URL. I’ll switch that to a short |
@SchrodingersGat |
|
The bounty is scoped to be generally useful, not just for one user #12397 (comment) |
|
To be clear, I am not saying that I do not want the links to be shareable - I simply had neglected to consider that in my previous review. With regard to shareable links, there is a consideration we need to make - the API does not always return the same results for each user. For example, there are many API endpoints which filter against "assigned_to_me" (or similar) - which will obviously return different results for each user That being said I am keen to see the links be shareable - but, perhaps there is a way to also make the URLs cleaner? |
URL ShorteningIn terms of URL shortening, let's use this example: Variable NamesThe variable names can be shortened, as an example
Nav FieldMost of the time, the nav field will be Nav APIThe |
|
There is some visual "instability" when clicking through the navigation items - see video below. It would be better if the buttons did not alter the height of the navigation bar. export-1786840143205.mp4 |
|
@Jr-kenny also, can you please add a documentation section into |
| field: '_nf' | ||
| } as const; | ||
|
|
||
| const LEGACY_DETAIL_NAVIGATION_PARAMS = { |
There was a problem hiding this comment.
This is not necessary, there is no "legacy" outside the context of this PR
| // Keep common built-in list endpoints compact while preserving the full URL | ||
| // for plugin and otherwise unknown endpoints. | ||
| const DETAIL_NAVIGATION_API_ALIASES: Record<string, string> = { | ||
| '/api/part/': 'p', |
There was a problem hiding this comment.
Use the existing ApiEndpoint enum rather than re-defining hard-coded API URLs
| {computedActions && ( | ||
| <Group gap={5} justify='right' wrap='nowrap' align='flex-start'> | ||
| {computedActions.map((action, idx) => ( | ||
| <Fragment key={idx}>{action}</Fragment> | ||
| ))} | ||
| {computedActions.length > 0 && ( | ||
| <Group gap={5} justify='right' wrap='nowrap' align='center'> | ||
| {computedActions} |
There was a problem hiding this comment.
why is this changed? wont this cause key issues?
| navCallback={breadcrumbAction} | ||
| breadcrumbs={computedBreadcrumbs} | ||
| breadcrumbs={computedBreadcrumbs ?? []} | ||
| rightSection={ |
There was a problem hiding this comment.
This should be in a seperate component, not the Breadcrumbs imo
| Object.values(LEGACY_DETAIL_NAVIGATION_PARAMS).forEach((key) => { | ||
| params.delete(key); | ||
| }); |
|
@matmair I pulled the detail navigation out of |




The detail navigation now follows the review feedback.
The helper is a
.tsxfile, and the navigation actions use the sharednavigateToLinkhandler. Previous and Next use Tabler icons with accessible labels and tooltips. A subtlex of yindicator shows the current position in the filtered result set, using the API count for the full total.I added a Playwright regression that opens a detail page from the table and moves from the first record to the second.
I checked this with
yarn tsc --noEmit,yarn build, Biome on the changed files, and a local Playwright browser flow against the demo dataset.Closes #12397