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

Try to detect language from html lang attributes and navigator.languages #38

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

wangcheng
Copy link
Contributor

@wangcheng wangcheng commented Mar 26, 2019

#37 # Description

  • rename "Locale" to "Language"
  • detect language from html lang attributes and navigator.languages

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

@codecov-io
Copy link

codecov-io commented Mar 26, 2019

Codecov Report

Merging #38 into master will decrease coverage by <.01%.
The diff coverage is 55.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #38      +/-   ##
==========================================
- Coverage   50.59%   50.59%   -0.01%     
==========================================
  Files         124      127       +3     
  Lines        1753     1777      +24     
==========================================
+ Hits          887      899      +12     
- Misses        866      878      +12
Impacted Files Coverage Δ
.../src/components/PlayerContainer/PlayerContainer.js 0% <ø> (ø) ⬆️
...es/griffith/src/contexts/Language/guessLanguage.js 100% <100%> (ø)
...iffith/src/contexts/Language/supportedLanguages.js 100% <100%> (ø)
.../griffith/src/contexts/Language/LanguageContext.js 100% <100%> (ø)
...th/src/components/TranslatedText/TranslatedText.js 100% <100%> (ø) ⬆️
...griffith/src/contexts/Language/LanguageProvider.js 14.28% <14.28%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a371cd...eb702b1. Read the comment docs.

@wangcheng wangcheng changed the title Locale improvements Detect language form html lang attributes and navigator.languages Mar 27, 2019
@wangcheng wangcheng changed the title Detect language form html lang attributes and navigator.languages Try to detect language from html lang attributes and navigator.languages Mar 27, 2019
@@ -24,15 +24,15 @@ const PlayerContainer = ({
children,
initialObjectFit = 'contain',
useMSE,
locale = 'en',
language,

Choose a reason for hiding this comment

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

是不是保留 locale 比较好,language 可能有歧义。

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

locale 和 language 还是不一样的。我们看起来只区分了语言,并没有区分「区域」,比如美国和英国词汇不同,数字格式不同之类的。

Copy link
Collaborator

Choose a reason for hiding this comment

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

我们这个属性可以填的值兼容了 locale 的写法,如果本意就是 locale 感觉写 locale 也没什么问题。美国英国的不同 locale 表现成同一种外观而已。

@xiaoyuhen xiaoyuhen added the enhancement✨ New feature or request label Apr 3, 2019
@kitayoshi
Copy link
Collaborator

这个的进度怎么样呀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement✨ New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants