Skip to content

Title and description in custom theme #349

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.

Sign up for GitHub

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

Jump to bottom
Closed
pavloko opened this issue May 6, 2018 · 8 comments
Closed

Title and description in custom theme #349

pavloko opened this issue May 6, 2018 · 8 comments
Labels
type: enhancement Request to enhance an existing feature

Comments

@pavloko
Copy link

pavloko commented May 6, 2018 •
edited
Loading

  • Your OS: MacOS 10.13.4
  • Node.js version : 9.4.0
  • VuePress version : 0.8.4
  • Is this a global or local install? : global install
  • Which package manager did you use for the install? : npm

When using a custom theme via .vuepress/theme/Layout.vue, config variables title and description specified in .vuepress/config.js are not rendered in the dist/index.html file.
@ulivz
Copy link
Member

ulivz commented May 6, 2018

Can you provide the repro repo?

@meteorlxy
Copy link
Member

meteorlxy commented May 6, 2018 •
edited
Loading

It's implemented in default theme, not in core.

Copy the related code from Layout.vue of default theme.

(So I proposed to extract them into mixins for better reuse #314 . Consider merging it? @ulivz 😃 )

vuepress/lib/default-theme/Layout.vue

Lines 90 to 96 in 5e87b65

created () {
if (this.$ssrContext) {
this.$ssrContext.title = this.$title
this.$ssrContext.lang = this.$lang
this.$ssrContext.description = this.$page.description || this.$description
}
},

Maybe we should put the code above to core, as it's necessary for the ssr template:

vuepress/lib/app/index.ssr.html

Lines 1 to 17 in 5e87b65

<!DOCTYPE html>
<html lang="{{ lang }}">
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width,initial-scale=1">
<title>{{ title }}</title>
<meta name="description" content="{{ description }}">
{{{ userHeadTags }}}
{{{ pageMeta }}}
{{{ renderResourceHints() }}}
{{{ renderStyles() }}}
</head>
<body>
<!--vue-ssr-outlet-->
{{{ renderScripts() }}}
</body>
</html>

@whoan
Copy link
Contributor

whoan commented May 7, 2018

@pavloko Do you have any locale set?

Notice that if you have something like this:

title: 'Hello',
description: 'World',
locales: {
  '/': {
    lang: 'en-US',
    title: 'VuePress',
    description: 'Vue-powered Static Site Generator'
  },
  '/zh/': {
    lang: 'zh-CN',
    title: 'VuePress',
    description: 'Vue 驱动的静态网站生成器'
  }
}

The locale will have precedence and that may be the reason of your problem...

It is worth mentioning that with the field description it even has more precedence any value in a page's Front Matter.

@ulivz
Copy link
Member

ulivz commented May 7, 2018

No any reproduced link, all the "conclusions" will be inconclusive.

@pavloko
Copy link
Author

pavloko commented May 7, 2018 •
edited
Loading

Hey guys, sorry for not providing this from the start. Here's the repo.

@whoan this is not the case, please check out the example.

@meteorlxy suggestion worked.

It was confusing that including head tags worked fine (you can see this in example repo), but title and description.

I see that #314 have been merged now (great work) and it seems that now to render title and description in a custom theme, Layout.vue should include something like:

import { updateMetaTags } from '@default-theme/mixins'

export default {
   mixins: [updateMetaTags]
}

However, coming back to the fact that config.head is included in the built despite any changes in Layout, is there any reason why we can't do the same for title and description? It seems a bit counterintuitive to include the mixin just to render title and description.

@meteorlxy
Copy link
Member

meteorlxy commented May 7, 2018 •
edited
Loading

@pavloko Yes, that's the point, so I mentioned:

Maybe we should put the code above to core, as it's necessary for the ssr template:

I would create a PR for that, but there is another problem, see #296 .

In #296, I wonder whether should we restrict $title in core.
In this issue, we are going to restrict this.$ssrContext.title = this.$title in core. Uh what a mess!

(Hmmm, #314 has not been merged yet, I think)

@pavloko
Copy link
Author

pavloko commented May 7, 2018

Hey @meteorlxy! Sorry, missed that line between the code blocks. I shared my thought in #296.

I'd agree that any head modifications should belong to core/config.

This was referenced May 9, 2018
Closed
Closed
Closed
@ulivz ulivz added 0.9.0 type: bug Something isn't working and removed need repro labels May 11, 2018
@ulivz
Copy link
Member

ulivz commented May 11, 2018 •
edited
Loading

@pavloko Now the title, description and lang will be rendered at not only default theme but also custom themes.

Fixed at fcaee80. and it will be released it at 0.9.0.

@ulivz ulivz added type: enhancement Request to enhance an existing feature and removed type: bug Something isn't working labels May 11, 2018
@ulivz ulivz closed this as completed May 11, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: enhancement Request to enhance an existing feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix: move ssrContext and updateMeta to core (#349, #370) meteorlxy/vuepress
4 participants
@whoan @pavloko @meteorlxy @ulivz