Multilanguage (but not only): Adding field menuitem to login menu item by infograf768 · Pull Request #10249 · joomla/joomla-cms

infograf768 · 2016-05-05T07:57:17Z

For a reason that I ignore (history I guess), the Login menu item contains a Redirect field in text format.
For multilanguage (as well as monolanguage), it is better to have a "menuitem" type field, which will prevent any error as the redirect should be internal. In that case it is an Itemid which is saved in the database (as already exists for the Logout menu item).
The issue here was to remain B/C while letting people use the alternative "menuitem" field.

To accomplish this, I added a new field of type "menuitem" (for the login and logout parts of the Login menu item), created 2 new rules, and added showon to let display only one field at a time and throw an alert preventing saving a value both in the ancient "Login Redirect" and the new "Menu Item Login Redirect" and as well for "Logout Redirect" and the new "Menu Item Logout Redirect".

When the menuitem field is used, the patch also checks if Multilang is on and, if it is, adds the necessary lang tag to the redirect.

Once this is merge, we will be able to merge a pending PR #9724 allowing multilanguage users to choose any redirection in the login module too, including associations when automatic change is on.

New interface (it is B/C as default is "Login Redirect" text field)

If choosing "Menu Item"

To test, enter internal values in the "Login redirect" and "Logout redirect" fields, save.
Then patch and test in monolanguage and multilanguage.

For example, if both fields are used for login (or/and logout), validation will prevent saving the menu item and we will get a Warning.

or

infograf768 · 2016-05-05T07:58:42Z

@andrepereiradasilva @brianteeman @richard67 @Bakual @mannybiker

Thanks for testing.

brianteeman · 2016-05-05T08:09:46Z

I have tested this item ✅ successfully on 6fa9124

Great stuff

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

brianteeman · 2016-05-05T08:13:49Z

Is it possible to make the default be the new menu item option and the custom url only displayed on load if it is set
that should still be b/c

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

infograf768 · 2016-05-05T08:21:06Z

@brianteeman
If we set the "Choose Login Redirect Type" field default to Menu Item (i.e. value="1", it will indeed not kill what was entered before in the present "Login Redirect" field, thus being B/C indeed. It may just confuse the user who already used that field.
It is an easy change. Let's see what other testers think.

joomla-cms-bot · 2016-05-05T08:26:15Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

brianteeman · 2016-05-05T08:26:55Z

But can it be set so that IF custom url not blank then it is displayed.
That should remove any confusion if possible

On 5 May 2016 at 09:21, infograf768 notifications@github.com wrote:

@brianteeman https://github.com/brianteeman
If we set the "Choose Login Redirect Type" field default to Menu Item
(i.e. value="1", it will indeed not kill what was entered before in the
present "Login Redirect" field, thus being B/C indeed. It may just confuse
the user who already used that field.
It is an easy change. Let's see what other testers think.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10249 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

infograf768 · 2016-05-05T08:29:12Z

let me see

infograf768 · 2016-05-05T11:19:27Z

nope, can'f find how to.
needs more tests and feedback

mannybiker · 2016-05-06T01:07:23Z

Item tested and working. Very useful addition since it was very time consuming and reason for mistakes to find all the correct article id, catid, itemid to compose manually the URL.

I agree with brianteeman on the default behaviour of having the menu item as default option if the custom URL filed is empty, but I understand that if this is technically difficult to achieve it is probably best to keep the Custom URL as the first option so that it is clear that either there is no configured URL or the old data is still there.

joomla-cms-bot · 2016-05-06T06:47:49Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

infograf768 · 2016-05-06T06:53:45Z

Found the solution. Easier in the morning 😄
The custom URL redirect fields will now display when the fields have a value. I obtained this result by adding a special case in the getItem() method in ROOT/administrator/components/com_menus/models/item.php
Menu Item is now default (test creating a new login menu item).
(Also took off a useless variable in the new rules files)

@mannybiker
@andrepereiradasilva
@brianteeman

Thanks for testing

brianteeman · 2016-05-06T07:05:22Z

Awesome - knew you wouldnt give up until you worked out how to do it

On 6 May 2016 at 07:53, infograf768 notifications@github.com wrote:

Found the solution. Easier in the morning 😄
The custom URL redirect fields will now display when the fields have a
value. I obtained this result by adding a special case in the getItem()
method in ROOT/administrator/components/com_menus/models/item.php
Menu Item is now default (test creating a new login menu item).
(Also took off a useless variable in the new rules files)

@mannybiker https://github.com/mannybiker
@andrepereiradasilva https://github.com/andrepereiradasilva
@brianteeman https://github.com/brianteeman

Thanks for testing

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10249 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

infograf768 · 2016-05-06T07:06:58Z

Can you folks set this RTC in issues (after testing 😃 )

brianteeman · 2016-05-06T07:08:12Z

Dont worry - I'm on a train to the sprint right now but will make sure it
gets merged over the weekend

On 6 May 2016 at 08:07, infograf768 notifications@github.com wrote:

Can you folks set this RTC in issues (after testing 😃 )

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#10249 (comment)

Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/

brianteeman · 2016-05-06T07:17:43Z

administrator/language/en-GB/en-GB.com_users.ini

 COM_USERS_FIELD_CATEGORY_ID_LABEL="Category"
 COM_USERS_FIELD_ID_LABEL="ID"
+COM_USERS_FIELD_LOGIN_MENUITEM="Menu Item"
+COM_USERS_FIELD_LOGIN_REDIRECT_CHOICE_DESC="'Custom URL' lets enter any internal url in the Redirect field while 'Menu Item' let's directly select an existing menu item.<br />For a multilingual site, it is advised to choose 'Menu Item'."


Can you change this string to the following as it is better English

+COM_USERS_FIELD_LOGIN_REDIRECT_CHOICE_DESC="'Custom URL' lets you manually enter any internal url in the Redirect field. 'Menu Item' lets you directly select an existing menu item.
For a multilingual site, it is advised to use 'Menu Item'."

brianteeman · 2016-05-06T07:20:12Z

Made a small suggestion for one of the language strings

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

joomla-cms-bot · 2016-05-06T07:26:18Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

infograf768 · 2016-05-06T07:26:42Z

Done. Thanks Brian.

brianteeman · 2016-05-06T07:39:12Z

Thanks

item in case some 3rd party component uses similar field names

joomla-cms-bot · 2016-05-06T07:56:47Z

This PR has received new commits.

CC: @brianteeman

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

andrepereiradasilva · 2016-05-06T12:08:05Z

Didn't do a full test, but discovered this issue.
Used sample data. Created a login menu item with this values.

Custom URL in login didn't work, the custom URL in logout worked.

andrepereiradasilva · 2016-05-06T12:14:42Z

Ok i see we have to add internal URI (index.php?....)... hum, this should be in the tooltip.

MATsxm · 2016-05-06T12:19:28Z

I have tested this item ✅ successfully on 5265043

Thanks

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

infograf768 · 2016-05-06T13:59:46Z

@andrepereiradasilva
the tip for the redirect url field is in main en-GB.ini. JFIELD_LOGIN_ etc. I did not touch it. It is unrelated to this PR

andrepereiradasilva · 2016-05-06T14:02:22Z

the tip for the redirect url field is in main en-GB.ini. JFIELD_LOGIN_ etc. I did not touch it. It is unrelated to this PR

i know, just pointing out that.

Tests in en-GB ok. Still have to test in a multilanguage env.

infograf768 · 2016-05-06T14:26:13Z

@andrepereiradasilva
I agree the tips for both redirect url are no good as they only say to enter an internal url without specifying the kind of format expected. (Which has created issues when we changed the code in 3.4.8 as the redirect should now be a JURI one.)
My proposal would be to let alone the constant and value in the main ini file and maybe change here all constants to new ones of type COM_USERS_ with better tip, and, as you said in our private chat, add in both fields a placeholder.
OR we could just add the placeholder.
What do you think?

andrepereiradasilva · 2016-05-06T14:27:30Z

i would add the new tooltip indicating that AND add placeholders (in all login/logout forms). But in another PR.

infograf768 · 2016-05-06T14:28:32Z

OK, let's do that in another PR and get this one in first.

andrepereiradasilva · 2016-05-06T15:52:17Z

I have tested this item ✅ successfully on 5265043

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

brianteeman · 2016-05-06T15:54:18Z

Rtc

_{This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.}

roland-d · 2016-05-07T10:55:43Z

Merged, thanks everybody for participating.

infograf768 · 2016-05-07T11:46:25Z

thanks @roland-d

… or login/logout menu items (#9724) * [imp] Multilanguage: allowing any login redirection * correcting menulogout to cope with logout set to Default * better * Implementing associations redirect from login module redirection * cs * Correcting when site is not multilingual * Taking off the part for menu logout as it is already in #10249

infograf768 · 2016-05-08T17:37:16Z

Strings modified here: #10336
please test

Adding field menuitem to login menu item

6fa9124

joomla-cms-bot added Language Change This is for Translators PR-staging labels May 5, 2016

infograf768 mentioned this pull request May 5, 2016

[imp] Multilanguage: allowing any login redirection from login module or login/logout menu items #9724

Merged

cs

87fd40d

Special case to display the correct redirect field.

97d09c2

brianteeman reviewed May 6, 2016
View reviewed changes

Modifying a language string

f65ea4b

Adding one more conditional to make sure we deal with a core Login Menu

5265043

item in case some 3rd party component uses similar field names

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 6, 2016

brianteeman added this to the Joomla 3.6.0 milestone May 6, 2016

infograf768 added a commit to infograf768/joomla-cms that referenced this pull request May 7, 2016

Taking off the part for menu logout as it is already in joomla#10249

34f5bbf

roland-d merged commit caadf91 into joomla:staging May 7, 2016

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label May 7, 2016

brianteeman added the Feature label May 8, 2016

brianteeman mentioned this pull request May 8, 2016

Inconsistency in menu item 'login' and module 'login' #7930

Closed

infograf768 deleted the login_menu_redirect branch May 11, 2016 06:19

infograf768 mentioned this pull request Apr 20, 2021

[4.0] [3.9] languagefilter automatic language change #32816

Closed

Uh oh!

Conversation

infograf768 commented May 5, 2016 • edited by brianteeman Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

infograf768 commented May 5, 2016

Uh oh!

brianteeman commented May 5, 2016

Uh oh!

brianteeman commented May 5, 2016

Uh oh!

infograf768 commented May 5, 2016

Uh oh!

joomla-cms-bot commented May 5, 2016

Uh oh!

brianteeman commented May 5, 2016

Uh oh!

infograf768 commented May 5, 2016

Uh oh!

infograf768 commented May 5, 2016

Uh oh!

mannybiker commented May 6, 2016

Uh oh!

joomla-cms-bot commented May 6, 2016

Uh oh!

infograf768 commented May 6, 2016

Uh oh!

brianteeman commented May 6, 2016

Uh oh!

infograf768 commented May 6, 2016

Uh oh!

brianteeman commented May 6, 2016

Uh oh!

brianteeman May 6, 2016

Choose a reason for hiding this comment

Uh oh!

brianteeman commented May 6, 2016

Uh oh!

joomla-cms-bot commented May 6, 2016

Uh oh!

infograf768 commented May 6, 2016

Uh oh!

brianteeman commented May 6, 2016 via email

Uh oh!

joomla-cms-bot commented May 6, 2016

Uh oh!

andrepereiradasilva commented May 6, 2016

Uh oh!

andrepereiradasilva commented May 6, 2016

Uh oh!

MATsxm commented May 6, 2016

Uh oh!

infograf768 commented May 6, 2016

Uh oh!

andrepereiradasilva commented May 6, 2016

Uh oh!

infograf768 commented May 6, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andrepereiradasilva commented May 6, 2016

Uh oh!

infograf768 commented May 6, 2016

Uh oh!

andrepereiradasilva commented May 6, 2016

Uh oh!

brianteeman commented May 6, 2016

Uh oh!

roland-d commented May 7, 2016

Uh oh!

infograf768 commented May 7, 2016

Uh oh!

infograf768 commented May 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

infograf768 commented May 5, 2016 •

edited by brianteeman

Loading

infograf768 commented May 6, 2016 •

edited

Loading