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.

Sign up for GitHub

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

Jump to bottom

fix dayjs i18n #755

Merged
merged 9 commits into from
Sep 21, 2020
Merged

fix dayjs i18n #755

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
90e8908
fix dayjs i18n
baurine Sep 8, 2020
ca9be18
refine
baurine Sep 17, 2020
f7e6697
Merge remote-tracking branch 'origin/master' into fix-dayjs-i18n
baurine Sep 17, 2020
192f3a6
Merge remote-tracking branch 'origin/master' into fix-dayjs-i18n
baurine Sep 17, 2020
bbcb94f
refine
baurine Sep 17, 2020
7fb8c2c
fix
baurine Sep 17, 2020
fdde546
fix
baurine Sep 17, 2020
bec06e2
refine
baurine Sep 17, 2020
328abbe
revert
baurine Sep 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion ui/lib/utils/i18n.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,11 @@ import { initReactI18next } from 'react-i18next'

i18next.on('languageChanged', function (lng) {
console.log('Language', lng)
dayjs.locale(lng.toLowerCase())
if (lng.startsWith('zh')) {
Copy link
Member

@breezewish breezewish Sep 14, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The whole logic becomes.. zh-CN -> zh -> zh-CN now. Is it better to revert some the changes in #677? IMO it is better to leave some capability to support both zhCN and zhTW. Will it work if we fallback zh to zh-CN?

https://www.i18next.com/principles/fallback#fallback-language

Copy link
Collaborator Author

@baurine baurine Sep 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Tried the fallback-language setting, it's a bit troublesome, you need to cover many languages, for example:

fallbackLng: { 
    'zh-HK': ['zh-CN', 'en'], 
    'zh-TW': ['zh-CN', 'en'],
    'zh-SG': ...,
    'zh-Hans': ...,
    'zh-Hant': ...,
    ...
}

According to some documents, the languages are also maybe:

zh-Hans 简体中文
zh-Hans-CN 大陆地区使用的简体中文
zh-Hans-HK 香港地区使用的简体中文
zh-Hans-MO 澳门使用的简体中文
zh-Hans-SG 新加坡使用的简体中文
zh-Hans-TW 台湾使用的简体中文
zh-Hant 繁体中文
zh-Hant-CN 大陆地区使用的繁体中文
zh-Hant-HK 香港地区使用的繁体中文
zh-Hant-MO 澳门使用的繁体中文
zh-Hant-SG 新加坡使用的繁体中文
zh-Hant-TW 台湾使用的繁体中文

(Although I don't meet)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think maybe we can fix this bug first. Then later if someone contributes the traditional Chinese translation for us, we can add the support for dayjs.

Copy link
Member

Choose a reason for hiding this comment

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

This fallback looks to be stupid. Why can't we just fallback zh to zh-CN?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Not sure whether zh -> zh-CN will make zh-TW fallback to zh-CN, let me have a try.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tried to revert it and use the following setting for i18next:

export const ALL_LANGUAGES = {
  'zh-CN': '简体中文',
  en: 'English',
}

i18next
  .use(LanguageDetector)
  .use(initReactI18next)
  .init({
    resources: {}, // oh! this line is a big pitfall, we can't remove it, else it will cause strange crash!
    fallbackLng: {
      zh: ['zh-CN'],
      default: ['en'],
    },
    interpolation: {
      escapeValue: false,
    },
  })

It works in most cases, except when you run the dashboard for the first time with another language likes zh-TW.

企业微信20200917112000

image

Because we don't import zh-tw locale for dayjs, and i18next's fallback doesn't affect dayjs.

But it will always works fine after manually selecting the 'zh-CN' or 'en' language.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I also tried this:

import en from 'dayjs/locale/en'
import zh from 'dayjs/locale/zh-cn'

//...

const DAYJS_LOCALES = { zh, en }
// console.log(DAYJS_LOCALES)
i18next.on('languageChanged', function (lng) {
  dayjs.locale(DAYJS_LOCALES[lng.toLocaleLowerCase()] || en)
})

It works when language is zh, but crashes when en, I think the dayjs has bug (will check it later).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Seems to find a good way:

import zh from 'dayjs/locale/zh-cn'

const DAYJS_LOCALES = { zh }

i18next.on('languageChanged', function (lng) {
  dayjs.locale(DAYJS_LOCALES[lng.toLocaleLowerCase()] || 'en')
})

Copy link
Member

@breezewish breezewish Sep 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe the real problem that #677 need to solve is that, the initial language is the detected language, instead of the effective language. So that by assigning the detected language it may not be a useful language.

Copy link
Collaborator Author

@baurine baurine Sep 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

So you mean we need write a util method getEffectiveLang() and use its value as Select component's default value, right?

I tried it and works fine. We don't need to revert the zh.yml to zh-CN.yml, and we shouldn't, else 'zh-TW' will fallback to 'en' translation instead of 'zh' translation.

export function getEffetiveLang(): string {
  const effetiveLangs = Object.keys(ALL_LANGUAGES)
  const detectedLang = i18next.language
  if (effetiveLangs.includes(detectedLang)) {
    return detectedLang
  }
  if (detectedLang.startsWith('zh')) {
    return 'zh-CN'
  }
  return 'en'
}

        <Form layout="vertical" initialValues={{ language: getEffetiveLang() }}>
          <Form.Item name="language" label={t('user_profile.i18n.language')}>
            <Select onChange={handleLanguageChange} style={{ width: 200 }}>
              {_.map(ALL_LANGUAGES, (name, key) => {
                return (
                  <Select.Option key={key} value={key}>
                    {name}
                  </Select.Option>
                )
              })}
            </Select>
          </Form.Item>
        </Form>

dayjs.locale('zh-cn')
} else {
dayjs.locale('en')
}
})

export function addTranslations(requireContext) {
Expand Down