[NEW] Add setLanguage method in livechat api methods by knrt10 · Pull Request #214 · RocketChat/Rocket.Chat.Livechat

knrt10 · 2019-04-16T19:16:55Z

TODO

Store language in iframe
Create a new file for language handling and handle all the task there

renatobecker-zz · 2019-04-23T18:32:17Z

src/components/App/index.js

 		await loadConfig();
 		this.handleTriggers();
 		CustomFields.init();
+		Language.init();


Why did you create a class to handle Language stuff instead of just exporting the methods you need to use?

renatobecker-zz · 2019-04-23T18:36:21Z

src/lib/hooks.js

@@ -1,5 +1,6 @@
 import Triggers from './triggers';
 import CustomFields from './customFields';
+import Language from './language';


You don't need a class to call just one method, you can create a file and export the method there.

renatobecker-zz · 2019-04-23T18:37:42Z

src/lib/hooks.js

 	},
+
+	setLanguage(language) {
+		Language.setLanguage(language);


Please, export the setLanguage method rather than instantiate a class to call the method.

renatobecker-zz · 2019-04-23T18:39:11Z

src/lib/language.js

@@ -0,0 +1,68 @@
+import store from '../store';
+
+class Language {


You don't need all of this stuff. The idea is to export the method(s) you want to call.
Please fix it.

knrt10 · 2019-04-24T06:19:13Z

Updated changes

renatobecker-zz · 2019-04-24T17:16:50Z

src/lib/language.js

+/**
+ * To get browser Language of user
+ */
+export const browserLanguage = () => {


You don't need to export this method.

renatobecker-zz · 2019-04-24T17:16:56Z

src/lib/language.js

+ * To normalize Language String and return language code
+ * @param {String} languageString
+ */
+export const normalizeLanguageString = (languageString) => {


You don't need to export this method.

renatobecker-zz · 2019-04-24T17:18:35Z

src/components/App/index.js

-			}
-
-			return countryCode ? `${ languageCode }_${ countryCode }` : languageCode;
+		const iframeLanguage = () => {


Since you're calling updateWidgetLanguage(), I think you don't need this block of code here, right?

renatobecker-zz · 2019-04-24T17:19:45Z

src/components/App/index.js

 		};

-		I18n.changeLocale(normalizeLanguageString(configLanguage() || browserLanguage()));
+		I18n.changeLocale(normalizeLanguageString(iframeLanguage() ? iframeLanguage() : configLanguage() || browserLanguage()));


Move this code into updateWidgetLanguage method.

renatobecker-zz · 2019-04-24T18:18:32Z

src/components/App/index.js

 		expanded,
 		alerts,
 		modal,
+		iframe,


Is this still necessary?

renatobecker-zz · 2019-04-24T18:20:17Z

src/lib/hooks.js

+
+	setLanguage(language) {
+		setLanguage(language);
+		updateWidgetLanguage();


Call this method inside setLanguage method.

renatobecker-zz · 2019-04-24T19:31:43Z

src/lib/language.js

+ */
+export const setLanguage = (language) => {
+	const { iframe } = store.state;
+	store.setState({ iframe: { ...iframe, language } });


Suggested change

store.setState({ iframe: { ...iframe, language } });

await store.setState({ iframe: { ...iframe, language } });

renatobecker-zz · 2019-04-24T19:32:23Z

src/lib/language.js

+ * This will update langauge of widget
+ */
+export const updateWidgetLanguage = () => {
+	const { iframe } = store.state;


Please, use Object destructuring below as well.

renatobecker-zz · 2019-04-24T19:35:57Z

src/lib/language.js

+ */
+export const updateWidgetLanguage = () => {
+	const { iframe } = store.state;
+	if (iframe && iframe.language) {


Improve this code. Instead of if/else, prepare the language value before calling I18n.changeLocale.
Please, improve the way you write the code.

renatobecker-zz · 2019-04-25T12:49:47Z

src/lib/language.js

+ * This will update langauge of widget
+ */
+export const updateWidgetLanguage = () => {
+	const { iframe: { language } = {} } = store.state;


Why not dealing with this information on configLanguage method?
You don't need to read the store so many times since you're reading the store on configLanguage method.
Please, test the iframe data on configLanguage method.

renatobecker-zz · 2019-04-25T12:50:26Z

src/lib/language.js

+ */
+export const updateWidgetLanguage = () => {
+	const { iframe: { language } = {} } = store.state;
+	I18n.changeLocale(language ? language : normalizeLanguageString(configLanguage() || browserLanguage()));


Suggested change

I18n.changeLocale(language ? language : normalizeLanguageString(configLanguage() || browserLanguage()));

I18n.changeLocale(normalizeLanguageString(configLanguage() || browserLanguage()));

Related to the comment I left above..

renatobecker-zz · 2019-04-25T12:51:27Z

src/lib/language.js

+/**
+ * This will update langauge of widget
+ */
+export const updateWidgetLanguage = () => {


Let's replace the name of this method by setWidgetLanguage.

renatobecker-zz · 2019-04-25T15:03:33Z

src/lib/language.js

+/**
+ * To get browser Language of user
+ */
+const browserLanguage = () => {


Suggested change

const browserLanguage = () => {

const browserLanguage = () => (navigator.userLanguage || navigator.language);

Use the correct arrow function statement. Your function does not have a return.

renatobecker-zz · 2019-04-25T15:08:57Z

src/lib/hooks.js

 		createOrUpdateGuest(data);
 	},
+
+	setLanguage(language) {


You don't need this method here, you can set the iframe language on the setLanguage method created on hooks.js.

knrt10 changed the title ~~[WIP] Add setLanguage method in livechat api methods~~ [NEW] Add setLanguage method in livechat api methods Apr 17, 2019

knrt10 force-pushed the setLanguage branch 2 times, most recently from 213950b to 9d593dd Compare April 17, 2019 17:57

renatobecker-zz suggested changes Apr 23, 2019

View reviewed changes

knrt10 force-pushed the setLanguage branch from b735512 to c360bf5 Compare April 24, 2019 13:23

renatobecker-zz suggested changes Apr 24, 2019

View reviewed changes

knrt10 force-pushed the setLanguage branch from 9d6db4c to 3dd67de Compare April 24, 2019 17:35

renatobecker-zz suggested changes Apr 24, 2019

View reviewed changes

knrt10 force-pushed the setLanguage branch from 3dd67de to 52e4811 Compare April 24, 2019 18:29

knrt10 mentioned this pull request Apr 24, 2019

Add new Livechat method and update department docs RocketChat/docs-old#1264

Merged

renatobecker-zz suggested changes Apr 24, 2019

View reviewed changes

knrt10 force-pushed the setLanguage branch from 52e4811 to f44208b Compare April 25, 2019 03:25

renatobecker-zz suggested changes Apr 25, 2019

View reviewed changes

knrt10 force-pushed the setLanguage branch 3 times, most recently from 243e33d to 9fef76a Compare April 25, 2019 13:29

renatobecker-zz suggested changes Apr 25, 2019

View reviewed changes

knrt10 added 6 commits April 25, 2019 22:06

added setLanguage method

784ce7e

added new lib for language

29e37c0

Class fix

3f56348

updated widget language

e5bf703

Spelling mistake

0efa18f

File name and some function changes

4f94372

knrt10 force-pushed the setLanguage branch from 050253f to 4f94372 Compare April 25, 2019 16:36

renatobecker-zz approved these changes Apr 25, 2019

View reviewed changes

renatobecker-zz merged commit 8086397 into RocketChat:dev Apr 25, 2019

This was referenced Apr 25, 2019

[IMPROVE] Extend Livechat API RocketChat/Rocket.Chat#13583

Closed

livechat: internationalization api #206

Closed

		@@ -0,0 +1,68 @@
		import store from '../store';

		class Language {

	store.setState({ iframe: { ...iframe, language } });
	await store.setState({ iframe: { ...iframe, language } });

	I18n.changeLocale(language ? language : normalizeLanguageString(configLanguage() \|\| browserLanguage()));
	I18n.changeLocale(normalizeLanguageString(configLanguage() \|\| browserLanguage()));

	const browserLanguage = () => {
	const browserLanguage = () => (navigator.userLanguage \|\| navigator.language);

Conversation

knrt10 commented Apr 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

knrt10 commented Apr 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renatobecker-zz Apr 24, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

renatobecker-zz Apr 25, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

knrt10 commented Apr 16, 2019 •

edited

Loading

renatobecker-zz Apr 24, 2019 •

edited

Loading

renatobecker-zz Apr 25, 2019 •

edited

Loading