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

Open routed URL in a new tab #339

Merged
merged 4 commits into from
Sep 18, 2023

Conversation

xavierfacq
Copy link
Member

Try to fix issue #272

@netlify
Copy link

netlify bot commented Sep 17, 2023

Deploy Preview for eclipsefdn-adoptium-dash ready!

Name Link
🔨 Latest commit c25d0d6
🔍 Latest deploy log https://app.netlify.com/sites/eclipsefdn-adoptium-dash/deploys/65073e90d8338200083925b0
😎 Deploy Preview https://deploy-preview-339--eclipsefdn-adoptium-dash.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@xavierfacq
Copy link
Member Author

It seems to work in the deploy preview:

https://deploy-preview-339--eclipsefdn-adoptium-dash.netlify.app/trends

I'll just try to add the correct select Tab depending off the current route.

@xavierfacq
Copy link
Member Author

xavierfacq commented Sep 17, 2023

✔️ Rewritings are goods

But it's more complicated than I thought to activate the matched route. I'll do than in another PR.

✔️ it's OK for the underline in the left menu

It doesn't build with my last modification. Let's just fix the rewriting for now

@xavierfacq xavierfacq marked this pull request as ready for review September 17, 2023 17:05
@xavierfacq xavierfacq marked this pull request as draft September 17, 2023 17:35
@xavierfacq xavierfacq marked this pull request as ready for review September 17, 2023 17:39
@xavierfacq xavierfacq marked this pull request as draft September 17, 2023 17:40
@xavierfacq xavierfacq force-pushed the fix/open_routed_url_in_a_new_tab branch from 8e297ed to c25d0d6 Compare September 17, 2023 17:59
@xavierfacq xavierfacq marked this pull request as ready for review September 17, 2023 18:02
@gdams

This comment was marked as outdated.

@gdams gdams linked an issue Sep 18, 2023 that may be closed by this pull request
@gdams gdams merged commit 3eaffbb into adoptium:main Sep 18, 2023
8 checks passed
@xavierfacq
Copy link
Member Author

@xavierfacq is there a reason you didn't go for @netomi's approach outlined here: #272 (comment)

I don't know HashRouter, but I know that rewriting works well and it was a quick fix for existing urls.

@netomi
Copy link

netomi commented Sep 18, 2023

I tested the implemented solution and it works nice. I think its a better approach than the HashRouter.

@xavierfacq xavierfacq deleted the fix/open_routed_url_in_a_new_tab branch October 5, 2023 14:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

https://dash.adoptium.net/trends cannot be visited in a new tab
3 participants