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: add i18n support #2075

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

feat: add i18n support #2075

wants to merge 9 commits into from

Conversation

NftTopBest
Copy link

@NftTopBest NftTopBest commented May 21, 2023

πŸ”— Linked issue

i18n feature issue item from docus: nuxt-themes/docus#770

❓ Type of change

  • πŸ“– Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • πŸ‘Œ Enhancement (improving an existing functionality like performance)
  • ✨ New feature (a non-breaking change that adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

πŸ“š Description

The PR adds i18n features.
The idea comes from here: nuxt-themes/docus#770
After some more deep digs, I find out the solution is to make a PR here. And nothing to do with the docus project.

Screenshot 2023-05-21 at 21 39 49

  • Add playground/i18n example
  • Add dev:i18n command in package.json

πŸ“ Checklist

  • I have linked an issue or discussion.
  • I have updated the documentation accordingly.

@nuxt-studio
Copy link

nuxt-studio bot commented May 21, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
content Edit on Studio β†—οΈŽ View Live Preview 2fa859d

@nuxt-studio
Copy link

nuxt-studio bot commented May 21, 2023

βœ… Live Preview ready!

Name Edit Preview Latest Commit
content Edit on Studio β†—οΈŽ View Live Preview 564d21e

@netlify
Copy link

netlify bot commented May 21, 2023

βœ… Deploy Preview for nuxt-content canceled.

Built without sensitive environment variables

Name Link
πŸ”¨ Latest commit 2fa859d
πŸ” Latest deploy log https://app.netlify.com/sites/nuxt-content/deploys/646b5c3abf9e7c00081cad85

Copy link
Contributor

Tahul commented May 22, 2023

@farnabaz @Atinux ;

This looks reasonable, I briefly checked the code and it seem to only be built for Document Driven mode.

As I think this behavior can easily be achieved via regular prefixed content queries, what do you think in including this in Document Driven mode?

We could also move that code somewhere in Docus I guess.

@Atinux
Copy link
Member

Atinux commented May 22, 2023

I don't think useI18n() is the correct name, I would at least name it useContentI18n() to avoid conflict with other libraries.

Before changing the document driven mode, I believe we need first to have an upgrade of it to avoid using a middleware but a plugin to inject the page to vue-router itself, this way we can leverage payload extraction as well as better i18n support.

@NftTopBest
Copy link
Author

NftTopBest commented May 22, 2023

We could also move that code somewhere in Docus I guess.

This is why we can not make it in Docus.

  1. At the beginning, I try to make it in Docus, then find out that I have to update the code here
  2. And finally all the code setup in Content is enough, no more code for Docus then.
  3. And only changing the code in Docus can not have the ability to update the navigation update to a new lang, check the code here: https://github.com/nuxt/content/pull/2075/files#diff-655e26ed4032bc6928b3b3838a699902e421b1838fadf34ed6d78decdfdfa69dR52-R64

The target features:

  1. user can have a lang switcher to switch to multiple langs with the top and sidebar nav change correspond
  2. user just simply setup the locales and defaultLocale in the nuxt.config.ts to turn on the feature in Docus or any other nuxt app that use the Content, there is no need to install the nuxt-i18n module
  3. nuxt-i18n module maybe not suit for this as it seems auto-generate new lang route for every lang, and in the content case

Just add an online demo here: https://nuxt-content-i18n.vercel.app/, pls check the result @Tahul @Atinux

@productdevbook
Copy link
Sponsor Member

This issue is extremely important to me, I hope it will be resolved for a moment. Thank you @NftTopBest pr.

@NftTopBest
Copy link
Author

This issue is extremely important to me, I hope it will be resolved for a moment. Thank you @NftTopBest pr.

@Tahul @Atinux Is that I need another update to make this PR works?

@truesteps
Copy link

Any progress?

@NftTopBest
Copy link
Author

NftTopBest commented Jun 7, 2023

@truesteps

Check out the demo here: https://nuxt-content-i18n.vercel.app/

For now, I am not sure what I should do further.

@truesteps
Copy link

@NftTopBest I love the implementation, I meant any progress on the maintainers side :) thanks for the quick work, it looks amazing

@XenBG
Copy link

XenBG commented Jun 16, 2023

I'd like to use that soon.

@Atinux Atinux requested a review from farnabaz June 16, 2023 08:00
@Barbapapazes
Copy link
Contributor

Whooo, awesome. Thanks for this! Can't wait to try it in my portfolio because it's currently written in French but many wants to read it in English.

@NftTopBest
Copy link
Author

Hi @farnabaz , is there anything I need to modify for the code?

@Barbapapazes
Copy link
Contributor

Barbapapazes commented Jul 4, 2023

Before changing the document driven mode, I believe we need first to have an upgrade of it to avoid using a middleware but a plugin to inject the page to vue-router itself, this way we can leverage payload extraction as well as better i18n support.

Hello @Atinux, could you explain more "inject the page to vue-router"? Thanks.

@Barbapapazes
Copy link
Contributor

Hi @farnabaz , is there anything I need to modify for the code?

Hello, I think that the support for i18n could also be add for non DocumentDriven mode. Using ContentDoc, ContentFetch and ContentNavigation.

@XeniaLu
Copy link

XeniaLu commented Aug 18, 2023

Hello, maintainers. May I inquire if there have been any recent updates regarding the timeline for merging this feature?

@mathieunicolas
Copy link
Contributor

Hi ! Thanks a lot for this PR, I hope it will be added to this project soon ! Can we do anything to help things going forward ? @farnabaz ?

Copy link

@DraftProducts DraftProducts left a comment

Choose a reason for hiding this comment

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

Another change is needed to support i18n : the parseMarkdown() function (L19-L33) should be called with some i18n options for the remark-rehype plugin :

const parsed = await parseMarkdown(content as string, {
      highlight: options.highlight,
      remark: {
        plugins: config.remarkPlugins
      },
      rehype: {
        options: {
+         footnoteLabel: 'Footprints'
          handlers: {
            link: link as any
          }
        },
        plugins: config.rehypePlugins
      },
      toc: config.toc
    }
})

ℹ️ The footnote option is documented here : https://github.com/remarkjs/remark-rehype#fields

The current way to configure without i18n is through the nuxt.config.ts file with redefining the plugin to force options :

content: {
  markdown: {
    remarkPlugins: [
      ['remark-rehype', {
        footnoteLabel: 'Footprints'
      }]
    ]
  }
},

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

Successfully merging this pull request may close these issues.

None yet

10 participants