-
Notifications
You must be signed in to change notification settings - Fork 218
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
Stop opening links in a new tab #4446
Conversation
7797fa8
to
3e1396e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Very interesting that our attribution HTML uses the same logic, but it makes sense 😄
I went back and forth on whether we should leave the attribution HTML with |
a1495cb
to
1ff34d0
Compare
Signed-off-by: Olga Bulat <obulat@gmail.com>
Signed-off-by: Olga Bulat <obulat@gmail.com> Update tests Signed-off-by: Olga Bulat <obulat@gmail.com>
1ff34d0
to
4179d60
Compare
Based on the medium urgency of this PR, the following reviewers are being gently reminded to review this PR: @dhruvkb Excluding weekend1 days, this PR was ready for review 7 day(s) ago. PRs labelled with medium urgency are expected to be reviewed within 4 weekday(s)2. @obulat, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes
Fixes #496 by @obulat
Description
This PR removes
target="_blank"
from the external links inVLink
component. This means that all external links will be opened in the same tab.There were 4 tests that were opening an external link, including the test that clicks on "Get audio". I updated them, and the unit tests, so I don't think we need a separate test just for checking if the link is opened in the same tab.
Testing Instructions
Run the app and try clicking on the external links: the navbar links such as "API", "get this media" links, license links. These links should open in the same tab, and some of them should still be sending "external click" analytics event.
Checklist
Update index.md
).main
) or a parent feature branch.just catalog/generate-docs
for catalogPRs) or the media properties generator (
just catalog/generate-docs media-props
for the catalog or
just api/generate-docs
for the API) where applicable.Developer Certificate of Origin
Developer Certificate of Origin