-
-
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
base: main
Are you sure you want to change the base?
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 [email protected] 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 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
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 config,toml
static/admin/config.yml
|
This works for me now. I have 2 concerns for now:
When we define a collection folder with
If we continue with option 1, we can take it even further, and we don't need a new Current Root This also adds the possibility of having the locale folder on the absolute root: @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 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:
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. |
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

...
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.
Do it for Piper :)