[#10249] - Multilanguage (but not only): Adding field menuitem to login menu item
- Fixed in Code Base
- 7 May 2016
- Medium
- Build: staging
- # 10249
- Diff
- infograf768:login_menu_redirect
- Success continuous-integration/travis-ci/pr The Travis CI build passed Details
- Success JTracker/HumanTestResults Human Test Results: 2 Successful 0 Failed. Details
User tests: Successful: Unsuccessful:
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
| Status | New | ⇒ | Pending |
| Labels |
Added:
?
?
|
||
| Labels |
Added:
?
?
|
||
| Labels | |||
| Labels | |||
| Category | ⇒ | Administration Language & Strings Multilanguage |
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.
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.
@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.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.
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/
let me see
nope, can'f find how to.
needs more tests and feedback
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.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.
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
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/brianteemanThanks 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/
Can you folks set this RTC in issues (after testing
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/
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.
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.
Done. Thanks Brian.
Thanks
This PR has received new commits.
CC: @brianteeman
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.
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.
Ok i see we have to add internal URI (index.php?....)... hum, this should be in the tooltip.
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.
@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
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.
@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?
i would add the new tooltip indicating that AND add placeholders (in all login/logout forms). But in another PR.
OK, let's do that in another PR and get this one in first.
I have tested this item successfully on 5265043
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/10249.
| Status | Pending | ⇒ | Ready to Commit |
| Labels | |||
| Labels |
Added:
?
|
||
| Milestone |
Added: |
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-05-07 10:54:39 |
| Closed_By | ⇒ | roland-d |
Merged, thanks everybody for participating.
| Labels |
Removed:
?
|
||
| Labels |
Added:
?
|
||
@andrepereiradasilva @brianteeman @richard67 @Bakual @mannybiker
Thanks for testing.