Skip to content

Add filtered detail navigation - #12585

Open
Jr-kenny wants to merge 12 commits into
inventree:masterfrom
Jr-kenny:Jrkenny/inventree-detail-navigation
Open

Add filtered detail navigation#12585
Jr-kenny wants to merge 12 commits into
inventree:masterfrom
Jr-kenny:Jrkenny/inventree-detail-navigation

Conversation

@Jr-kenny

@Jr-kenny Jr-kenny commented Aug 8, 2026

Copy link
Copy Markdown

The detail navigation now follows the review feedback.

The helper is a .tsx file, and the navigation actions use the shared navigateToLink handler. Previous and Next use Tabler icons with accessible labels and tooltips. A subtle x of y indicator 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

@netlify

netlify Bot commented Aug 8, 2026

Copy link
Copy Markdown

Deploy Preview for inventree-web-pui-preview ready!

Name Link
🔨 Latest commit c63fec9
🔍 Latest deploy log https://app.netlify.com/projects/inventree-web-pui-preview/deploys/6a81e2d4310a14000804e99b
😎 Deploy Preview https://deploy-preview-12585--inventree-web-pui-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 97 (🟢 up 1 from production)
Accessibility: 81 (no change from production)
Best Practices: 100 (no change from production)
SEO: 78 (no change from production)
PWA: -
View the detailed breakdown and full score reports
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@Jr-kenny
Jr-kenny marked this pull request as ready for review August 8, 2026 13:12
@matmair matmair added the User Interface Related to the frontend / User Interface label Aug 11, 2026
@matmair matmair added this to the 1.6.0 milestone Aug 11, 2026
@matmair matmair self-assigned this Aug 11, 2026
Comment thread src/frontend/src/functions/DetailNavigation.tsx
Comment thread src/frontend/src/hooks/UseDetailNavigation.tsx Outdated
@SchrodingersGat

Copy link
Copy Markdown
Member

User Interface Buttons

I would like to see the navigation buttons being icons (tabler) - not text. Also, a subtle x of y text might be helpful. See example image below

image

@SchrodingersGat

Copy link
Copy Markdown
Member

Unit Tests

This will require regression testing via playwright.

@SchrodingersGat

Copy link
Copy Markdown
Member

@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

image

@SchrodingersGat

Copy link
Copy Markdown
Member

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 ?nav=1A2BC3

@matmair

matmair commented Aug 13, 2026

Copy link
Copy Markdown
Member

@SchrodingersGat that makes it not shareable, invalidating most of the usefulness of links

@SchrodingersGat

Copy link
Copy Markdown
Member

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.

@Jr-kenny

Copy link
Copy Markdown
Author

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 nav hash like ?nav=1A2BC3, with the navigation data stored in sessionStorage under that hash. I’ll update the link builder, reader, and Playwright coverage as part of that.

@Jr-kenny

Jr-kenny commented Aug 13, 2026

Copy link
Copy Markdown
Author

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.

@SchrodingersGat
Just to confirm before I change it umm should I go ahead with the sessionStorage + short ?nav= hash approach for local navigation, even though those links won’t be shareable? Or do we want to keep the navigation state in the URL so links remain portable?

@matmair

matmair commented Aug 13, 2026

Copy link
Copy Markdown
Member

The bounty is scoped to be generally useful, not just for one user #12397 (comment)

@SchrodingersGat

Copy link
Copy Markdown
Member

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?

@SchrodingersGat

SchrodingersGat commented Aug 16, 2026

Copy link
Copy Markdown
Member

URL Shortening

In terms of URL shortening, let's use this example: http://localhost:5173/web/manufacturing/build-order/7/details?_nav_api=%2Fapi%2Fbuild%2F&_nav_index=17&_nav_pk=7&_nav_field=pk&_nav_query=part_detail%3Dtrue%26outstanding%3Dtrue

Variable Names

The variable names can be shortened, as an example

Long Name Short Name
_nav_api _na
_nav_query _nq
_nav_index _ni

Nav Field

Most of the time, the nav field will be pk - you can omit that unless it is not pk

Nav API

The _nav_api values are mostly going to point to hard-coded fixed text values - we could have a lookup table for these to shorten the URL even further? e.g. _nav_api=XYZ

@SchrodingersGat

Copy link
Copy Markdown
Member

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

@SchrodingersGat

Copy link
Copy Markdown
Member

Also, please put the text first, before the buttons, so that the position of the buttons does not move as the length of the text changes:

image

@SchrodingersGat

Copy link
Copy Markdown
Member

@Jr-kenny also, can you please add a documentation section into docs/concepts/ui.md detailing the new feature

field: '_nf'
} as const;

const LEGACY_DETAIL_NAVIGATION_PARAMS = {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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',

@SchrodingersGat SchrodingersGat Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Use the existing ApiEndpoint enum rather than re-defining hard-coded API URLs

@SchrodingersGat

Copy link
Copy Markdown
Member

@Jr-kenny nice, getting very close I think. Review needed from @matmair

Comment on lines +187 to +258
{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}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why is this changed? wont this cause key issues?

navCallback={breadcrumbAction}
breadcrumbs={computedBreadcrumbs}
breadcrumbs={computedBreadcrumbs ?? []}
rightSection={

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This should be in a seperate component, not the Breadcrumbs imo

Comment on lines +102 to +104
Object.values(LEGACY_DETAIL_NAVIGATION_PARAMS).forEach((key) => {
params.delete(key);
});

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is not needed IMO

@matmair matmair left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this is the most promising PR for sure; as this is a new function these legacy catches against previous iterations of the PR are unncessary

@Jr-kenny

Copy link
Copy Markdown
Author

@matmair I pulled the detail navigation out of BreadcrumbList. PageDetailNavigationBar now owns the shared bar and renders BreadcrumbList and DetailNavigation as siblings, so the controls stay in the same visual position without the breadcrumb component owning them. Is this the separation you had in mind?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

is_bounty User Interface Related to the frontend / User Interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR] Add Next and Previous Icon to switch easily between 2 objects

3 participants