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

feat(i18n): nav dropdown language selector #91

Merged
merged 7 commits into from
Oct 19, 2020
Merged

feat(i18n): nav dropdown language selector #91

merged 7 commits into from
Oct 19, 2020

Conversation

Spice-Z
Copy link
Contributor

@Spice-Z Spice-Z commented Sep 19, 2020

At #50, @antfu implemented i18n support, and said.

The only thing left is nav dropdown, that would be another PR.

I created that!

Oct-01-2020 20-42-52

Working config is like this

...
themeConfig: {
 locales: {
    '/': {
      label: 'English',
      selectText: 'Languages',
    },
    '/zh/': {
      label: '中文',
      selectText: '选择语言',
    },
    '/ja/': {
      label: '日本語',
      selectText: '言語,
    },
  },
}

(To set config about site title, description, and lang, you have to use global locale config. Check #50)

Comment on lines 68 to 69
// TODO i18n text
text: 'Languages',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This text also needs i18n

Copy link
Contributor Author

Choose a reason for hiding this comment

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

DONE

@Spice-Z Spice-Z marked this pull request as ready for review September 19, 2020 08:35
Copy link
Member

@kiaking kiaking left a comment

Choose a reason for hiding this comment

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

Thank you so much for the feature! Looking really good!

I have 2 requests.

Currently, VuePress locale config sits inside themeConfig. Would it be possible to move the new configs under there instead of locales?

{
  themeConfig: {
    locales: {
      '/': {
        label: 'English',
        selectText: 'Languages',
        editLinkText: 'Edit this page on GitHub',
    },
    ...
  }
}

Ref: https://vuepress.vuejs.org/guide/i18n.html#default-theme-i18n-config

  1. Can we move i18n dropdown before GitHub repo link at Nav?

@@ -7,6 +7,7 @@
</template>
</template>
<NavBarLink v-if="repoInfo" :item="repoInfo" />
<NavDropdownLink v-if="localeCandidates" :item="localeCandidates" />
Copy link
Member

Choose a reason for hiding this comment

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

Lets move this one above the GitHub repo nav 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved!

@Spice-Z
Copy link
Contributor Author

Spice-Z commented Oct 11, 2020

@kiaking
I fixed both of your requests!

About first request, I didn't notice vitepress( and vuepress) has two kinds of locales in config...!
(locales is used as Site Level i18n Config, themeConfig.locals is used as Theme i18n Config.)

Now, this selector will use themeConfig.locals's label and selectText

Copy link
Member

@kiaking kiaking left a comment

Choose a reason for hiding this comment

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

Thanks a lot! Just one more thing. I found a bug that this PR will not work on configs that does not contain any locales info. Please see the inline comment I made 🙏

@Spice-Z
Copy link
Contributor Author

Spice-Z commented Oct 18, 2020

@kiaking
Thank you !
Sorry to late commit..., I fixed it!

I have to think of any every case of config, or have to write test code. 😅

Copy link
Member

@kiaking kiaking left a comment

Choose a reason for hiding this comment

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

Great! Thanks a lot. Tested at my side and it is working well 🙌

@kiaking kiaking merged commit 294836c into vuejs:master Oct 19, 2020
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants