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: changed minimum of 2 locales to minimum of 1 #6406

Merged
merged 10 commits into from
Aug 23, 2023
Merged

Conversation

dfdez
Copy link
Contributor

@dfdez dfdez commented Apr 13, 2022

Summary
I made these PR in order to be able to have only one locale, to cover some use cases in which we want to just have one.

Right now the only workaround is to just set i18n: false but this will change how the file / folder structure is generated

closes #6263

Test plan

The only modification that I made is to set the minItems property of locales property from 2 to 1.

Also I added a test for check that fails when locales are less than 1 (Previously there wasn't a test of this)

Checklist

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • Code is formatted via running yarn format.
  • Tests are passing via running yarn test.
  • The status checks are successful (continuous integration). Those can be seen below.

A picture of a cute animal (not mandatory but encouraged)

image

@dfdez dfdez marked this pull request as ready for review April 13, 2022 21:13
@dfdez dfdez requested a review from a team April 13, 2022 21:13
@erezrokah erezrokah added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Apr 14, 2022
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

Thanks @dfdez, I'm concerned that we might rely of that array to contain more than 1 element in a few places in the codebase.

For example with this change the CMS crashes in EditorControlPane.js
image

Do you mind looking at the error?

@dfdez
Copy link
Contributor Author

dfdez commented Apr 14, 2022

Thanks @dfdez, I'm concerned that we might rely of that array to contain more than 1 element in a few places in the codebase.

For example with this change the CMS crashes in EditorControlPane.js image

Do you mind looking at the error?

Sure!

@dfdez
Copy link
Contributor Author

dfdez commented Apr 14, 2022

The problem was the i18n editor toggle, by having only 1 locale the editor tried to load locale[1] creating the problem showed above.

In a52ca73 I updated the EditorInterface.js to only show the toggle if i18n is enabled and there are multiple locales.

@dfdez dfdez requested a review from erezrokah April 14, 2022 11:16
@dfdez
Copy link
Contributor Author

dfdez commented Apr 22, 2022

Any news about this? @erezrokah

@martinjagodic martinjagodic requested review from demshy and removed request for erezrokah April 25, 2023 12:05
@dfdez dfdez closed this May 12, 2023
@martinjagodic
Copy link
Member

@dfdez any reason why you closed this? This would be great to implement after we solve priority issues.

@dfdez
Copy link
Contributor Author

dfdez commented May 12, 2023

I thought it was abandoned 🥲 but It can be reopened of course!

@martinjagodic martinjagodic reopened this May 12, 2023
@martinjagodic
Copy link
Member

Not abandoned, just in a long queue :D

@martinjagodic
Copy link
Member

@dfdez can you solve merge conflicts, please? They appeared because packages and folders were renamed.

@demshy demshy merged commit f3a30b5 into decaporg:master Aug 23, 2023
20 checks passed
martinjagodic pushed a commit that referenced this pull request Oct 16, 2023
* feat: changed minimum of 2 locales to minimum of 1

* feat: only show i18n editor toggle if there are multiple locales
martinjagodic pushed a commit that referenced this pull request Oct 17, 2023
* feat: changed minimum of 2 locales to minimum of 1

* feat: only show i18n editor toggle if there are multiple locales
martinjagodic pushed a commit to geotrev/netlify-cms that referenced this pull request Oct 17, 2023
* feat: changed minimum of 2 locales to minimum of 1

* feat: only show i18n editor toggle if there are multiple locales
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pinned type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow i18n to have only one locale
4 participants