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

languages improved + fix repeated js in discussions #170

Merged
merged 3 commits into from
Feb 12, 2018
Merged

languages improved + fix repeated js in discussions #170

merged 3 commits into from
Feb 12, 2018

Conversation

galdazbiz
Copy link
Contributor

I hope you better like this way to integrate the titles.
Additionally chatter.js was duplicated in discussions when you select the sidebar in the config, now solved.
I have also new views, mobile friendly, I changed it 30%, not sure if to create a pull request, add them as new "vendor" template..

@galdazbiz
Copy link
Contributor Author

I also added a "if" in discussion and home to be able to configure the alerts externally in case there's a system in the template already managing all the alerts from the site.

no need for array for a simple 'errors' => false,
@galdazbiz
Copy link
Contributor Author

not merged yet??

@marktopper
Copy link
Collaborator

Thanks

@marktopper marktopper merged commit b6a8b78 into thedevdojo:master Feb 12, 2018
@gerbenjacobs
Copy link
Contributor

I have two questions.

  • Shouldn't the errors attribute config be set to "true" by default?
  • Why are we using this strtolower(trans('chatter::intro.titles.discussion')) in the translation files? If you now set it to "Discussions" in the config, all languages will use that word.

@marktopper
Copy link
Collaborator

marktopper commented Feb 12, 2018

Shouldn't the errors attribute config be set to "true" by default?

Nice catch, fixed in #177

Why are we using this strtolower(trans('chatter::intro.titles.discussion')) in the translation files? If you now set it to "Discussions" in the config, all languages will use that word.

You might want to translate Discussions to the other languages as well.

@gerbenjacobs
Copy link
Contributor

gerbenjacobs commented Feb 12, 2018

But in that case you need:

chatter::intro.titles.discussion.en = Discussion
chatter::intro.titles.discussion.es = Discusión
chatter::intro.titles.discussion.nl = Discussie

Also I just found out there's a problem with UTF-8 and strtolower. This needs to become mb_strtolower to keep multi-byte/encoding issues at bay.

Although my preference is to not use Config settings in translations at all.
Edit: Ah yes, I was thinking about the old situation.

@galdazbiz
Copy link
Contributor Author

galdazbiz commented Feb 12, 2018

@gerbenjacobs

You're right with the "true" errors

Regarding using 'chatter::intro.titles.discussion' this word is taken from a file in lang, not in config, it was like that before, but I changed it in this fix, because I agree better not to use config for words.

And yes, mb_strtolower might be better, I use that because in some languages you can capitalise all the words in a sentence, but in others is not correct.

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