-
-
Notifications
You must be signed in to change notification settings - Fork 5.7k
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: support relative path with target config. #1751
Conversation
This pull request is being automatically deployed with Vercel (learn more). docsify-preview – ./🔍 Inspect: https://vercel.com/docsifyjs/docsify-preview/HRJhhNznrse57qin19594z9xQ1FA |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 628690e:
|
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.
One minor stylistic question, otherwise looks good. :)
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!
remove console.
Why do we check if it doesn't start with I'm of the opinion that we shouldn't get in the way, should not change intent. So if someone writes a mailto link with a target, just like they can in regular HTML ( It in other words, it shouldn't be our goal to change DOM conventions, but only to provide the interface to it as much as we can. |
@trusktr --
I don't know the history, but my guess is that the original contributor was unaware of the usefulness of using the
Agreed, but this is a separate issue. My vote would be to review this PR for its intended purpose and create another PR to "fix" the target attribute on |
@trusktr Hi, as @jhildenbiddle said, it is the original contributor did, I wanna keep it incase of any unpredicted changes. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@Koooooo-7 -- Since this has been targeted for v5, can we update this branch and get one more review/approval? @sy-records? @trusktr? |
Summary
Support relative path set
target
config.close #1750
What kind of change does this PR introduce?
Bugfix
For any code change,
Does this PR introduce a breaking change? (check one)
If yes, please describe the impact and migration path for existing applications:
Related issue, if any:
#1750
Tested in the following browsers: