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

Menu external url #476

Merged
merged 5 commits into from
Dec 10, 2018
Merged

Menu external url #476

merged 5 commits into from
Dec 10, 2018

Conversation

jiangtj
Copy link
Member

@jiangtj jiangtj commented Nov 13, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our guidelines.
  • Tests for the changes have been added (for bug fixes / features).
    • Muse | Mist have been tested.
    • Pisces | Gemini have been tested.
  • Docs have been added / updated (for bug fixes / features).

PR Type

What kind of change does this PR introduce?

  • Bugfix.
  • Feature.
  • Code style update (formatting, local variables).
  • Refactoring (no functional changes, no api changes).
  • Build related changes.
  • CI related changes.
  • Documentation content changes.
  • Other... Please describe:

What is the current behavior?

Menu didn't support jump to external links

Issue Number(s): N/A

What is the new behavior?

Menu support jump to external links
image
click google jump to www.google.com

Config in the end of this file
Previews:

How to use?

In NexT _config.yml:

  menu:
    home: / || home
    archives: /archives/ || archive
    google: https://www.google.com || google
    more:
      default: /more/ || info
      google: http://www.google.com || google
      more:
        google: https://www.google.com || google
        default: /more/ || info

Note
default is not supported, due to default is used for displaying some submenu info.

Does this PR introduce a breaking change?

  • Yes.
  • No.

@@ -21,7 +21,11 @@
{% if itemName == 'default' %}
{% set parentValue = subvalue.split('||')[0] | trim %}
{% else %}
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is a problem with submenu
can u check this WITH Getting Started? https://test.saili.science/docs/

menu:
  home: / || home
  #about: /about/ || user
  categories: /categories/ || th
  archives: /archives/ || archive
  tags: /tags/ || tags
  #schedule: /schedule/ || calendar
  #sitemap: /sitemap.xml || sitemap
  #commonweal: /404/ || heartbeat
  #navi: /navi/ || paper-plane-o
  #hits: /hits/ || fire
  #comments: /comments/ || comment-o
  Docs:
    default: /docs/ || book
    Getting Started:
      default: https://github.com/theme-next/hexo-theme-next || flag

@jiangtj
Copy link
Member Author

jiangtj commented Nov 29, 2018

Yes, it didn't work on default.
But I think that default is used for display some submenu info, it shouldn't start with https://.
If you want Getting Started as a link to outside site, can write directly.

  Docs:
    default: /docs/ || book
    Getting Started: https://github.com/theme-next/hexo-theme-next || flag

@sli1989
Copy link
Collaborator

sli1989 commented Nov 29, 2018

well, i understand.
it's better to add explanatory note about this.

@jiangtj
Copy link
Member Author

jiangtj commented Nov 29, 2018

😂 I don't know where to add explanatory note, because there's no note about submenu.

@ivan-nginx
Copy link
Member

@jiangtj just add here in this PR something like readme. Later it will be on main site.

@jiangtj
Copy link
Member Author

jiangtj commented Nov 29, 2018

@ivan-nginx okay,I updated this PR, Add note in How to use

@ivan-nginx
Copy link
Member

@jiangtj maybe #463 PR (v6.6.0) already solved this problem. Please, check it.

@jiangtj
Copy link
Member Author

jiangtj commented Dec 10, 2018

@ivan-nginx this PR didn't solve it.

@ivan-nginx
Copy link
Member

@jiangtj can you give some explanation on it, what currently didn't work? Because on your test site «Google» links work fine.

@jiangtj
Copy link
Member Author

jiangtj commented Dec 10, 2018

@ivan-nginx
In my test site, deploy two-branch, it merge menu-external-url and fix-same-submenu
Current Behavior:
image

@jiangtj
Copy link
Member Author

jiangtj commented Dec 10, 2018

This json is generated in deploy. It show deploy repo and branch info
https://jiangtj.gitlab.io/next-test/deploy.json

@ivan-nginx
Copy link
Member

Ok, i check and confirm it. But i remember what in #463 PR i tested «Google» (external) links and they working fine.

  1. Maybe Fix #495, #473 and #231 url_for() misuse #496 PR make this bug?
  2. On your site you use latest master branch or latest release (v.6.6.0) version?

@jiangtj
Copy link
Member Author

jiangtj commented Dec 10, 2018

@ivan-nginx
image
You may only test outermost menu

@ivan-nginx
Copy link
Member

So, the only problem for this PR:

  1. Main menu work with «Google» links.
  2. Submenu and sub-submenu not work with «Google» links?

@jiangtj
Copy link
Member Author

jiangtj commented Dec 10, 2018

Emmm... For main menu, generated link is https:/www.google.com, it lost / and it isn't correct link.
But on most browser, work 'fine'...

@ivan-nginx ivan-nginx added this to the v6.7.0 milestone Dec 10, 2018
@ivan-nginx ivan-nginx merged commit 87f515c into theme-next:master Dec 10, 2018
@ivan-nginx
Copy link
Member

@jiangtj thanks for bugfixes for NexT!

@jiangtj jiangtj deleted the menu-external-url branch December 10, 2018 16:05
tongluyang pushed a commit to tongluyang/hexo-theme-next that referenced this pull request Nov 19, 2019
* Add menu ext url support

* Add doc

* Simplified protocol condotion & Added exturl style in for Pisces/Gemini.

* Added `.exturl` condition to span tag.
@Brannua
Copy link

Brannua commented May 8, 2022

Hi, may i ask a question ? I'm using the latest version of Next.

My site URL is xxx.com, and i want to set xxx.com/sub as a external_link.

However, because of the domain is not changed, so the xxx.com/sub is not open in a new tab.

If there is way to open xxx.com/sub in a new tab when i click it in menu ?

@Brannua
Copy link

Brannua commented May 8, 2022

@ivan-nginx Is there any solutions ? thanks very much.

wens07 pushed a commit to wens07/hexo-theme-next that referenced this pull request May 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants