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

Links to metabase URLs should open in the same window in link cards #41508

Merged
merged 4 commits into from Apr 18, 2024

Conversation

heypoom
Copy link
Contributor

@heypoom heypoom commented Apr 17, 2024

Partially fixes #30891

Description

External link cards currently always opens in new tabs. When clicking on a link to metabase urls, such as https://example.com/question/1-some-question, it opens a new tab instead of opening in the same window. This makes the URL break out of embedding as well.

This PR makes it so links with the same origin or same as the site url open in the same window.

Caveat

This also causes all relative links to open in the same tab. This would make relative links to questions and dashboards open correctly, but invalid relative links such as example.com, www.example.com, foobar will remain broken. An open question is how should we handle invalid relative links in external link cards.

How to verify

  • Create a new link card in a dashboard.
  • Enter the absolute url to a metabase question or dashboard, e.g. https://localhost:3000/dashboard/1-example
  • Click on the card, and it should open in the same page. Go back to the dashboard.
  • Next, edit the card and enter a relative url to a metabase question or dashboard, e.g. dashboard/1-example
  • Click on the card, and it should also open in the same page

Checklist

  • Tests have been added/updated to cover changes in this PR

@heypoom heypoom added the backport Automatically create PR on current release branch on merge label Apr 17, 2024
@heypoom heypoom marked this pull request as ready for review April 17, 2024 10:59
@metabase-bot metabase-bot bot added the visual Run Percy visual testing label Apr 17, 2024
Copy link

github-actions bot commented Apr 17, 2024

Codenotify: Notifying subscribers in CODENOTIFY files for diff c288e4d...1374c28.

Notify File(s)
@alxnddr frontend/src/metabase/visualizations/visualizations/LinkViz/LinkViz.tsx
frontend/src/metabase/visualizations/visualizations/LinkViz/LinkViz.unit.spec.tsx

Copy link

replay-io bot commented Apr 17, 2024

Status Complete ↗︎
Commit 1374c28
Results
⚠️ 12 Flaky
2397 Passed

Copy link
Member

@WiNloSt WiNloSt left a comment

Choose a reason for hiding this comment

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

This works great 👍

Re:

An open question is how should we handle invalid relative links in external link cards.

As they're already broken, I don't think we have to do anything. The only thing we change here is to open them in the same tab rather than on the new page, but the behavior of the open pages remains the same.

I think users would likely be able to spot broken links and fix them themselves pretty easily.

@heypoom heypoom enabled auto-merge (squash) April 18, 2024 09:08
@heypoom heypoom merged commit e21201c into master Apr 18, 2024
110 checks passed
@heypoom heypoom deleted the fix-30891-link-card-urls branch April 18, 2024 09:32
Copy link

@heypoom Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

github-actions bot pushed a commit that referenced this pull request Apr 18, 2024
…41508)

* external link cards should apply the correct target attribute

* add unit tests for absolute and relative question links in link cards
metabase-bot bot added a commit that referenced this pull request Apr 18, 2024
…41508) (#41566)

* external link cards should apply the correct target attribute

* add unit tests for absolute and relative question links in link cards

Co-authored-by: Phoomparin Mano <poom@metabase.com>
@heypoom heypoom added this to the 0.49.7 milestone Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/Embedding visual Run Percy visual testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Overall link situation
2 participants