Skip to content

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

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

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

Conversation

matmair
Copy link
Contributor

@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 User Interface Related to the frontend / 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
Contributor 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 ed2ce0e
🔍 Latest deploy log https://app.netlify.com/projects/inventree-web-pui-preview/deploys/6854449e006de3000845b67e
😎 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: 95 (no change 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

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

Copy link

codecov bot commented Dec 28, 2024

Codecov Report

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

Project coverage is 85.53%. Comparing base (8493e5a) to head (acf67a1).

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 0.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.55%   85.53%   -0.02%     
==========================================
  Files        1177     1177              
  Lines       52041    52082      +41     
  Branches     2120     2120              
==========================================
+ Hits        44523    44549      +26     
- Misses       6984     6987       +3     
- Partials      534      546      +12     
Flag Coverage Δ
pui 69.51% <0.00%> (+0.28%) ⬆️

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
Contributor 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
Contributor Author

matmair commented Dec 29, 2024

@SchrodingersGat added

@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
Contributor 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

@matmair
Copy link
Contributor Author

matmair commented Feb 10, 2025

Not really sure what to do about the middle click, I can not find where it is getting it's link in that case and why it is diefferent

@sur5r
Copy link
Contributor

sur5r commented Feb 10, 2025

Middle click doesn't work at all for me. It's not surprising given the main menu items are no links (a href) but buttons. I guess this is just a clash of worlds. This is no longer web browsing, so we can't expect it to behave as such.

@matmair
Copy link
Contributor Author

matmair commented Feb 11, 2025

that is the thing - there should be/is an a element rendered - so it should work the same as "normal" html

@SchrodingersGat
Copy link
Member

@matmair it looks like most of the effects of this PR have been reverted, not sure if that is intentional? But the <a> elements are no longer in play

@matmair
Copy link
Contributor Author

matmair commented Apr 18, 2025

I have re-introduced the rendering as elements; seems to work but I do not have a middle click for debug
would appreciate a review @sur5r @SchrodingersGat

@sur5r
Copy link
Contributor

sur5r commented Apr 18, 2025

The links are there, but one doesn't end up on the correct view/subpage. It neither works with middle click nor with "copy link" and pasting the link into a new tab.

Example: Open a part and use the link from e.g. "Part pricing" to open an new tab. It opens the Part, but on "Part Details".

It seems to URLs are generated incorrectly, i.e. they point to /part/301/details/pricing instead of /part/301/pricing.

@matmair
Copy link
Contributor Author

matmair commented Apr 20, 2025

Seems to work on my machine - are we talking about the same thing?

Screen.Recording.2025-04-19.at.15.54.31.mov

@sur5r
Copy link
Contributor

sur5r commented Apr 20, 2025

No, I'm talking about these links:

image

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 User Interface Related to the frontend / User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants