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.

Sign up for GitHub

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

Jump to bottom

Fixing Header to rely on location and not parameter. #6472

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fixing Header to rely on location and not parameter. #6472

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
43b8b1b
Updated the Header tabValue to rely on the location pathname instead …
CodingPupper3033 Feb 12, 2024
39d7fda
Fixing my quotation marks?
CodingPupper3033 Feb 12, 2024
fa1cb1b
Alphabeticalizing my imports!!!!
CodingPupper3033 Feb 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/frontend/src/components/nav/Header.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useDisclosure } from '@mantine/hooks';
import { IconBell, IconSearch } from '@tabler/icons-react';
import { useQuery } from '@tanstack/react-query';
import { useState } from 'react';
import { useNavigate, useParams } from 'react-router-dom';
import { useLocation, useNavigate } from 'react-router-dom';

import { api } from '../../App';
import { navTabs as mainNavTabs } from '../../defaults/links';
Expand Down Expand Up @@ -100,12 +100,13 @@ export function Header() {

function NavTabs() {
const { classes } = InvenTreeStyle();
const { tabValue } = useParams();
const defaultTabValue = 'home';
const tabValue = useLocation().pathname.split('/')[1] || defaultTabValue;
Copy link
Member

@matmair matmair Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure that is a great solution, this is indirectly hardcoded to the current schema - which might change at any point. Thoughts @wolflu05 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this would also be my first concern, as we have the feature to configure a relative path where PUI is hosted. Maybe we can use that setting to somehow strip the prefix or find a solution to only get the url from after the prefix.

And secondly it would be better to wrap the .split and parsing of the panel name into a useMemo hook, so this is in a state and does not cause a rerendering.

Copy link
Contributor Author

@CodingPupper3033 CodingPupper3033 Feb 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I agree with your concerns, and it may not be the best idea for this application. If you do come up with a solution, let me know (@ me). I would love to see how you guys fix it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another method I tried (and failed to get to work) was to replace the path in the top-level route (<Route path="/" element={<LayoutComponent />} errorElement={<ErrorPage />}>) with something like path="/tabValue?", so the value of the child roots would be filled into that value, and theoretically still work, but in practice, it either absorbs everything, or breaks children routing.

It might be worth a shot? I'm not qualified enough in React to know if it's possible.

const navigate = useNavigate();

return (
<Tabs
defaultValue="home"
defaultValue={defaultTabValue}
classNames={{
root: classes.tabs,
tabsList: classes.tabsList,
Expand Down
Loading