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

Allow relative links in footer. #3254

Conversation

taylorreece
Copy link
Contributor

Motivation

I have a big write-up on Stack Overflow about what I'm trying to do: https://stackoverflow.com/questions/63268853/how-do-i-link-to-non-docusaurus-defined-routes-from-a-docusuarus-footer

The TL;DR is that my Docusaurus docs are compiled and static files are saved to a /docs directory of my webserver, but the rest of my site is developed outside of Docusaurus. I'd like to link to local resources like /contact-us or /about-us from my Docusaurus footer, but /about-us is currently an "invalid" href value, and if I try to use the to: '/about-us' syntax, the React router tries to snag my click.

This change allows for such local links by adding the allowRelative flag to the URI validator. https://github.com/sideway/joi/blob/master/API.md#stringurioptions

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

I verified that I could add a footer link of the form:

            {
              label: 'About Us',
              href: '/about-us',
            },

and the config validator didn't complain, and the react router didn't try to snag my click.

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Aug 10, 2020
@docusaurus-bot
Copy link
Contributor

docusaurus-bot commented Aug 10, 2020

Deploy preview for docusaurus-2 ready!

Built with commit 8acd93e

https://deploy-preview-3254--docusaurus-2.netlify.app

@slorber
Copy link
Collaborator

slorber commented Aug 11, 2020

Hi @taylorreece

Sorry but not really willing to introduce this change, as we expect href to be URLs with domains.

2 solutions you have:

{
              label: 'About Us',
              href: process.env.MY_DOMAIN + '/about-us',
},
{
              label: 'About Us',
              to: '/about-us',
              target: '_blank' 
},

@slorber slorber closed this Aug 11, 2020
@taylorreece
Copy link
Contributor Author

Hey @slorber ,

Thanks for your feedback! It got me on the right track to get a hack in place. I figured I'd document here what I found, in case anyone else runs into this problem:

Unfortunately for me, the first option,

{
              label: 'About Us',
              href: process.env.MY_DOMAIN + '/about-us',
},

doesn't work for me since MY_DOMAIN is not known at compile time. Our CI/CD pipeline builds artifacts, and then pushes those artifacts to our QA, staging, and prod stacks, each of which have different domains.

The second option,

{
              label: 'About Us',
              to: '/about-us',
              target: '_blank' 
},

didn't seem to work for me either. Since my baseUrl is /docs/ I'm linked to /docs/about-us instead of /about-us. Plus, it opens up a new window anyways which is undesirable.

I tried to find a way to link the current window, so naturally turned to target: "_self". The react router seems to snag the link when I have target: "_self", however if I do:

            {
              label: "About Us",
              to: "../about-us/",
              target: "_parent"
            },

That seems to override the react router "capture click" behavior, and takes care of the base url stuff by going up a directory. So.... it creates a link that the react router doesn't capture, but also links to the correct place. That'll work for me for now.

Cheers,
Taylor

@slorber
Copy link
Collaborator

slorber commented Aug 11, 2020

great you find a solution

I know the linking story is not ideal currently but I try to avoid rushing on a solution too fast until I understand better this problem space

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants