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

Normalize locale input format before matching, fix #3 #4

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PeterDaveHello
Copy link

  • Repace - with _
  • Capitalize characters after _

Original results looks odd in some situations:

> FacebookLocales.bestFacebookLocaleFor('en-IN')
'en_GB'
> FacebookLocales.bestFacebookLocaleFor('zh-TW')
'zh_CN'

The corrected results would be as fellow:

> FacebookLocales.bestFacebookLocaleFor('en-IN')
'en_IN'
> FacebookLocales.bestFacebookLocaleFor('zh-TW')
'zh_TW'

- Repace `-` with `_`
- Capitalize characters after `_`
@@ -60,6 +60,14 @@ const localesToNonStandardFacebookLocales = _(facebookVirtualLocales)
.value()

export const bestFacebookLocaleFor = locale => {
// Normalize locale input format
if (locale.includes('-')) {

Choose a reason for hiding this comment

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

The if (locale.includes('-')) { check is moot for this as replace is a noop if there's no - in the string.

Copy link
Author

Choose a reason for hiding this comment

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

I treat this just about kind of style, I know the results is totally the same :)

locale = locale.replace('-', '_');
}
if (locale.includes('_')) {
locale = locale.split('_')[0] + '_' + locale.split('_')[1].toUpperCase();

Choose a reason for hiding this comment

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

This can be written more readable:

const [language, territory] = locale.split('_');
locale = `${language}_${territory.toUpperCase()}`;

Copy link
Author

@PeterDaveHello PeterDaveHello May 26, 2018

Choose a reason for hiding this comment

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

I'm not sure which style is more acceptable, would like to wait for review from its maintainer.

@PeterDaveHello
Copy link
Author

@dleshem do you mind to help review this PR? Thanks!

@dleshem
Copy link
Contributor

dleshem commented Dec 27, 2020

@PeterDaveHello Sorry, I no longer work in Wix...

@PeterDaveHello
Copy link
Author

@PeterDaveHello Sorry, I no longer work in Wix...

Sorry, didn't notice that 😅

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.

3 participants