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

Implement Localization. #43

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

Implement Localization. #43

wants to merge 12 commits into from

Conversation

andreyjan
Copy link

No description provided.

Copy link

vercel bot commented Aug 19, 2024

@andreyjan is attempting to deploy a commit to the FFW Sandbox Team on Vercel.

To accomplish this, @andreyjan needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

Copy link

vercel bot commented Aug 19, 2024

An owner of the FFW Sandbox Team on Vercel accepted @andreyjan's request to join.

@andreyjan's commit is now being deployed.

Copy link

vercel bot commented Aug 19, 2024

You must have Developer access to commit code to FFW Sandbox on Vercel. If you contact an administrator and receive Developer access, commit again to see your changes.

Learn more: https://vercel.com/docs/accounts/team-members-and-roles/access-roles#team-level-roles

Copy link
Collaborator

@asgorobets asgorobets left a comment

Choose a reason for hiding this comment

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

That's a great start!

Can you take a look at my comments and also consider adding a language switcher (dropdown or links, dropdown preferred)
Example from Marketing Starterkit is fine: https://marketing-site.templates.contentful.com/?referrer=contentful

export default async function LandingPage({ params }: { params: { slug: string[] } }) {
export default async function LandingPage({ params }: { params: { slug: string[]; locale: string } }) {
const { locale } = params;
setStaticParamsLocale(locale);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see setStaticParamsLocale but don't see getStaticParams() in generateStaticParams, isn't the point of setStaticParamsLocale to help generate pages for all locales? Documentation is pretty unclear on that, but maybe looking at source code can help?

Copy link
Author

Choose a reason for hiding this comment

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

You are correct. I refactored so now we generate pages fro combinations slug + locale.

@@ -0,0 +1,13 @@
import { ReactElement } from 'react';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not convinced a sublayout is a good idea here, shouldn't we wrap the original top level layout with I18nProviderClient as well? I suspect if SiteNavigation or Header would have client components that need translations, those would not work with this setup.

It seems that folks in next-international in general moved all their top level files to [locale]/ folder.
https://github.com/QuiiBz/next-international/blob/main/examples/next-app/app/%5Blocale%5D/layout.tsx

I'm just thinking we should add a client-component example and try to test it, also not a huge fan of moving all files to [locale] subfolder but maybe that's what we might have to do if we see problems with other approaches

Copy link
Author

Choose a reason for hiding this comment

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

Implemented the structure as in the example and moved the localization provider to the root layout.

Copy link

vercel bot commented Aug 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nextjs-contentful ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 22, 2024 0:15am

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.

Implement Localization
2 participants