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.

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

feat(PUI): Make header tabs links to simpilfy new tab behaviour #8779

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

Conversation

matmair
Copy link
Member

@matmair matmair commented Dec 27, 2024

This PR enables opening header tab items per right-click to new tabs.

@matmair matmair added enhancement This is an suggested enhancement or new feature Platform UI Related to the React based User Interface labels Dec 27, 2024
@matmair matmair added this to the 1.0.0 milestone Dec 27, 2024
@matmair matmair self-assigned this Dec 27, 2024
@matmair
Copy link
Member Author

matmair commented Dec 27, 2024

@sur5r 👀

Copy link

netlify bot commented Dec 27, 2024

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

Name Link
🔨 Latest commit ae96dc5
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/677d2411e06e9f0008182e0c
😎 Deploy Preview https://deploy-preview-8779--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: 99 (🔴 down 1 from production)
Accessibility: 86 (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

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

Copy link

codecov bot commented Dec 28, 2024

Codecov Report

Attention: Patch coverage is 33.33333% with 4 lines in your changes missing coverage. Please review.

Project coverage is 83.72%. Comparing base (3a62bdd) to head (ae96dc5).

Files with missing lines Patch % Lines
...rc/frontend/src/components/buttons/AdminButton.tsx 0.00% 0 Missing and 1 partial ⚠️
src/frontend/src/components/nav/SearchDrawer.tsx 0.00% 0 Missing and 1 partial ⚠️
src/frontend/src/components/panels/PanelGroup.tsx 50.00% 0 Missing and 1 partial ⚠️
src/frontend/src/functions/navigation.tsx 0.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8779      +/-   ##
==========================================
- Coverage   85.17%   83.72%   -1.45%     
==========================================
  Files        1178     1131      -47     
  Lines       51807    50557    -1250     
  Branches     2092     1928     -164     
==========================================
- Hits        44125    42329    -1796     
- Misses       7141     7776     +635     
+ Partials      541      452      -89     
Flag Coverage Δ
pui 58.63% <33.33%> (-10.79%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SchrodingersGat
Copy link
Member

@matmair this is a nice addition. It would make sense to add it for the "Panel" nav tab links too?

@matmair
Copy link
Member Author

matmair commented Dec 28, 2024

Propaply; should this be included in this patch?

@SchrodingersGat
Copy link
Member

@matmair yes I think so, should be only very minimal diff

@matmair
Copy link
Member Author

matmair commented Dec 29, 2024

@SchrodingersGat added

@@ -66,7 +66,7 @@ export default function AdminButton(props: Readonly<AdminButtonProps>) {
`${server.server.django_admin}${modelDef.admin_url}${props.id}/`
);

if (event?.ctrlKey || event?.shiftKey) {
if (event?.ctrlKey || event?.shiftKey || event?.metaKey) {
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is used over and over again, maybe it makes sense to extract this into a custom function?

Copy link
Member Author

Choose a reason for hiding this comment

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

As those are seperate functions/interactions I would keep them seperate for now

@SchrodingersGat
Copy link
Member

@matmair I have noticed something here - the target link is different depending on how the click event is interpreted:

Starting Point

From this starting point, I will click the "Manufacturing" tab:

image

Left Mouse Button

Navigates to "Manufacturing" page in same tab

image

Left Mouse Button + Control Key

Opens "Manufacturing" page in new tab

image

Middle Mouse Button

Opens "Manufacturing" page (via demo site) in new tab:

image

Note that the URL in the address bar is different in this case!

The same behaviour hapens with the side panels too...

@matmair
Copy link
Member Author

matmair commented Jan 2, 2025

@SchrodingersGat I am not familiar with middle button. Which control key is this?

@SchrodingersGat
Copy link
Member

I am not familiar with middle button. Which control key is this?

No control key, just click on the link by pressing the scroll wheel on the mouse. Typically this just opens a link in a new tab (similar to ctrl+click).

However, it bypasses the browser onClick event system somehow so we can't catch it like a normal click. Somehow, this type of click uses a different baseUrl when generating the link target

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This is an suggested enhancement or new feature Platform UI Related to the React based User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants