-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add different i18n path structure #7400
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
Conversation
Hello @martinjagodic, or whoever can answer this for us:
Is there any kind of timeline we can expect for this PR to be reviewed?
Asking because we need to get the url structure working for our site and are facing deadlines. The answer for what we can expect here will help us decide where we should allocate our development resources accordingly.
Thank you in advance!
@offscriptdev sorry, there is no timeline. We review PRs when we find the time. Currently we have some other PRs in the pipeline to merge, so I can't promise you anything.
If you really need this to happen, you can reach out to decap@p-m.si and we can discuss this as a priority.
Thanks @martinjagodic for your quick reply. It's very helpful.
I tried to deploy this myself, and got as far as building the JS for the front-end. Saving the content works as expected with all files being saved in the correct location. However, on refresh, the content disappears from the UI.
I tracked this down to the folder argument passed into allEntriesByFolder
method on the implementation type. By default the folder will be /content/blog which gets all locales and then filters them by the default locale. So, all we need to do is pass in the default locale to the folder we're fetching. I got this fixed and working locally for Github, but I see there are other implementations.
I'll see what I can do about patching all the other implementations a little later.
...oot for query to server
I've got the UI fetching the content from Gtihub and the local server without issues. I can get a screen recording of my tests if needed, as I didn't see any automated tests covering this.
I'd test other implementations, like Gitlab, but I'm frankly way too busy.
Recent changes from this repo have been merged into my fork.
@offscriptdev can you share the folder structure that should work with this? preferably a minimal reproduction repository. I tried to set one up, but I didn't get any entries in the collection view.
my folder structure
/en
-- /posts
---- _index.md
---- article.md
/sl
-- /posts
---- _index.md
---- article.md
my config
i18n: structure: multiple_folders_i18n_root locales: [sl, en] collections: - name: posts i18n:true folder: posts
Thanks @martinjagodic I set up a minimal reproduction for you here: https://github.com/offscriptdev/decap-i18n-at-root-example
content structure:
image
config,toml
baseURL = ''
defaultContentLanguage = 'en-gb'
title = 'Example'
[languages]
[languages.en-gb]
contentDir = 'content/en-gb'
languageName = 'English'
languageCode = 'en-GB'
[languages.fr-fr]
contentDir = 'content/fr-fr'
languageName = 'Français'
languageCode = 'fr-FR'
static/admin/config.yml
backend:
name: git-gateway
branch: main
media_folder: static/media
i18n:
structure: multiple_folders_i18n_root
locales: [en-gb, fr-fr]
default_locale: en-gb
collections:
- name: "blog"
label: "Blog"
folder: "content/blog"
create: true
i18n: true
slug: "{{year}}-{{month}}-{{day}}-{{slug}}"
editor:
preview: true
fields:
- { label: "Title", name: "title", widget: "string", i18n: true }
- { label: "Body", name: "body", widget: "markdown", i18n: true }
This works for me now. I have 2 concerns for now:
multiple_folders_i18n_root
is very long. I suggest locale_folders
as a shorter alternative.
When we define a collection folder with folder: content/posts
, we omit the information that between content
and posts
, there is the locale level. I see 2 possible alternatives, which are not necessarily better:
- We define a template tag to have something like
folder: content/{{ locale }}/posts
- We define the content folder at the root level:
content_folder: content
, and at the collection level, we only definefolder: posts
.
If we continue with option 1, we can take it even further, and we don't need a new structure
option; we can extend the multiple_folders
option. I imagine it like this:
Current multiple_folders
: folder: content/posts/{{locale}}
. If the locale tag is missing, always append it to the end.
Root multiple_folders
(what this PR does): folder: content/{{locale}}/posts
.
This also adds the possibility of having the locale folder on the absolute root: folder: {{locale}}/content/posts
@offscriptdev, what do you think?
@martinjagodic I think the hardest part of programming isn't making the code work, but deciding what variables should be named. I can never decide between things like let isOkay = false
or just let ok = false
. So, whatever name you decide is appropriate for this is good by me.
You're right that the convention for defining collection folders could use improvement. My apologies for not making this better out of the box. We were in the middle of trying to launch an i18n feature and needed something that just worked, so I went with the simple, yet inelegant, solution.
I think option #1 would be the best in the long run. Most developers should be familiar with the {{variable}} syntax to insert a value into a string, and doing things in this way yields the following benefits:
- It's the clearest to understand where the locale will be, so future support requests should decline
- It's the most flexible, so future feature requests should decline too
Maybe it's a bit more work, but I think the juice is worth the squeeze. I'd be happy to help out with this later in my spare time, but right now my timeline for availability with spare time is a good month or two out.
Uh oh!
There was an error while loading. Please reload this page.
Summary
We have an overall i18n path strategy to place the locale just after the root, and decap does not support this out of the box. To fix this, we added a new structure to i18n.ts.
Basically, Decap supports this site.com/content/locale/slug
But we needed it to support this site.com/locale/content/slug
Test plan
npm run test && npm run format are passing
image
image
...
image
Checklist
Please add a
x
inside each checkbox:A picture of a cute animal (not mandatory but encouraged)
If you don't approve this PR, Piper will be sad.
image
Do it for Piper :)