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

Fix #891 - Adding site-wide and locale-specific support for changing the logo link #1015

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

dylburger
Copy link

@dylburger dylburger commented Nov 16, 2018

Summary

This PR adds support for the feature requested in #891 : adding the option to change the link to which the user is directed when clicking the logo.

I ran across this issue on my own VuePress site: I'm keeping the default base URL (/), but have no content at /, only at /terms, /privacy, etc. We actually host a completely different app at the root of our domain, and handle routing to that app using Cloudfront cache behaviors. Specifically,

Since the router-link tied to the logo currently does client-side routing to /, and since I have no content at /, users get a 404. Since I'm handling routing at the Cloudfront-level, I need to direct users to the external URL pointing them to our app (in the example above, https://domain.com).

My use case was to change the logoLink on a site-wide level, so I've added support for including a URL at $site.themeConfig.logoLink. In the absence of this property, the logo link falls back to the $localePath (existing behavior).

I also thought it would be nice to add locale-specific logo links, so I first check for the presence of $themeLocaleConfig.logoLink, then $site.themeConfig.logoLink, then $localePath.

I've tested this manually but extensively. Since this is my first VuePress commit, I'm sure I'm missing something, so I'm definitely open to feedback.

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

All tests done on macOS 10.14

  • Chrome 70.0.3538.77
  • Firefox 63.0.3 (64-bit)
  • Safari Version 12.0 (14606.1.36.1.9)
  • Edge
  • IE

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated (see below)

I didn't see any existing tests for the relevant code, but I'm happy to add / update those tests if you'd like.

@rajaraodv
Copy link

Thanks, @dylburger !

@ulivz ulivz force-pushed the master branch 3 times, most recently from 6c3127f to 71574f2 Compare December 18, 2018 18:27
@ulivz ulivz force-pushed the master branch 5 times, most recently from 316e022 to 1284944 Compare January 29, 2019 11:47
@flozero flozero added complexity: easy Easy complexity topic: config Relates to VuePress config topic: theme Relates to VuePress theme type: enhancement Request to enhance an existing feature labels Sep 5, 2019
@flozero
Copy link
Collaborator

flozero commented Sep 5, 2019

Hello @dylburger thank's for your pr i know it's been a while a create the pr. Sorry for that.

Can you please resolve the conflict ?

Thank's for your time !

@flozero flozero self-requested a review September 5, 2019 14:40
@flozero flozero added the version: 1.x Relates to version 1 of VuePress label Sep 5, 2019
@ulivz
Copy link
Member

ulivz commented Dec 20, 2021

Conflict here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: easy Easy complexity topic: config Relates to VuePress config topic: theme Relates to VuePress theme type: enhancement Request to enhance an existing feature version: 1.x Relates to version 1 of VuePress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants