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

Allow override of inherited CMS language when in CiviCRM #17006

Merged
merged 1 commit into from
May 4, 2020

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Apr 7, 2020

Overview

If inheriting the CMS language you cannot change the language in CiviCRM backend anymore using lcMessages=en_GB URL parameter (which sets the language for the session).
With "inherit CMS language" contribution pages etc. track the CMS language which is usually a good thing. But if you, for example, have an extra language on the backend that is not available on the frontend you cannot select it.

Also, you cannot select which languages should be "available" because the available languages selector is hidden. This means that you a) have to pre-download any languages you might use. b) Can only limit the available CiviCRM languages by deleting the language files. c) Can't use an extension like l10nupdate to automatically download language files.

Before

  • Inherit CMS language prevents overriding CiviCRM language via lcMessages parameter.
  • Inherit CMS language prevents selecting which languages are available in CiviCRM.

After

  • Inherit CMS language still allows overriding CiviCRM language via lcMessages parameter.
  • Inherit CMS language prevents selecting which languages are available in CiviCRM.

Technical Details

  • Check if sessionLocale is empty before inheriting CMS locale.
  • Simplify the localization settings by removing javascript so we never hide the uiLanguages select field.

Comments

@mlutfy @MikeyMJCO thoughts please?

@civibot civibot bot added the master label Apr 7, 2020
@civibot
Copy link

civibot bot commented Apr 7, 2020

(Standard links)

@mlutfy
Copy link
Member

mlutfy commented Apr 15, 2020

I think it's generally OK, but we should double-check any help texts to make sure it's not too confusing. For example, the help text on "inherit language" says:

"If checked, CiviCRM will follow CMS language changes."

and this is not really true anymore if we merge this PR. One could change the language using the language switcher, then use the CMS language switcher, and be really confused about why this is not changing the language?

We should tweak to "CiviCRM will set the initial session language, which can later be changed if using the language switcher", but few admins will see it or remember that when using CiviCRM.

ping @demeritcowboy (relates to issues you raised recently about the lcMessages param, which we want to deprecate since not Angular-friendly)

@demeritcowboy
Copy link
Contributor

We should tweak to "CiviCRM will set the initial session language, which can later be changed if using the language switcher", but few admins will see it or remember that when using CiviCRM.

I think that tweak makes sense. The inherit from CMS option currently seems backwards to me, i.e. is there a reason, other than history, that it couldn't be "just inherit, and then allow an override in civi"? By using a language switcher for example - which is related to the other ticket about possibly making it a native menu option instead of by url or drupal-only block (which uses url).

By the way I did start taking a look at the kamlanguage extension but got sidetracked.

@mattwire
Copy link
Contributor Author

@demeritcowboy @mlutfy It now looks like this:
image

Hopefully that is ok? I switched the field to use metadata instead of hardcoding descriptions in the template.

@demeritcowboy
Copy link
Contributor

Something weird with karma tests aborting. Jenkins test this please.

@homotechsual
Copy link
Contributor

homotechsual commented Apr 17, 2020

@demeritcowboy
Copy link
Contributor

Noting that a request for comments had been put out in chat on the 17th by @mlutfy: https://chat.civicrm.org/civicrm/pl/mdzrygkzmbfx8f8h9g3rodyz8w and no objections seem to be noted.

@aydun
Copy link
Contributor

aydun commented Apr 30, 2020

When language switcher was separated from multi-language, we let 'Inherit CMS language' cause language switcher to be hidden to limit confusion for those already managing multiple languages via the CMS. Providing the precedence of the CMS and Civi settings are clear and documented, this change looks good.

@mattwire
Copy link
Contributor Author

mattwire commented May 4, 2020

Docs issue here civicrm/civicrm-user-guide#443

@mattwire
Copy link
Contributor Author

mattwire commented May 4, 2020

Merging based on approval from @aydun @demeritcowboy @mlutfy

@mattwire mattwire merged commit e7684f0 into civicrm:master May 4, 2020
@mattwire mattwire deleted the l10nallowoverrideinherit branch May 4, 2020 11:45
@eileenmcnaughton
Copy link
Contributor

According to #17919 (comment) this caused a regression

@mattwire
Copy link
Contributor Author

@eileenmcnaughton Yes, it's been a bit of a game of whackamole - me and @christianwach had a chat about the language issues and concluded that the only way to fix things properly is to make sense of and completely refactor the applyLocale() function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants