Problem/Motivation
When I add a new link in a menu and I select the parent link. The option list with the links appears truncated.
The function that truncate the title with a maximum of 30 characters is parentSelectOptionsTreeWalk
in core/lib/Drupal/Core/Menu/MenuParentFormSelector.php
. (Line 163)
$title = $indent . ' ' . Unicode::truncate($link->getTitle(), 30, TRUE, FALSE);
Does this limit exist for some special reason? And what would be the best way to override it?
Steps to reproduce
From #19
Steps to test -
1. Go to the admin site.
2. Go to admin/structure/menu/manage/admin/add?destination=/drupal9.1.x-dev/admin/structure/menu.
3. Create a menu link.
4. Again Go to admin/structure/menu/manage/admin/add?destination=/drupal9.1.x-dev/admin/structure/menu.
5. Verify the parent link field.
Proposed resolution
#24 :
we should just remove the truncation altogether.
Before:
After:
Remaining tasks
Write tests
User interface changes
None
API changes
None
Data model changes
None
Release notes snippet
None
Comment | File | Size | Author |
---|---|---|---|
#58 | test.png | 38.92 KB | quietone |
#56 | interdiff_55-56.txt | 583 bytes | jnlar |
#56 | 1166282-56.patch | 2.06 KB | jnlar |
| |||
#55 | 1166282-55.patch | 2.08 KB | jnlar |
#55 | interdiff_51-55.txt | 991 bytes | jnlar |
Comments
Comment #1
juampynr CreditAttribution: juampynr commentedSubscribing. I would like to know the reason too.
Comment #2
Vacilando CreditAttribution: Vacilando commentedA rather annoying issue. In dropdowns with just a few nested menus titles quickly become unintelligible.
Also unsure what's the best way to override this -- ideas?
Comment #3
taxicab221 CreditAttribution: taxicab221 commentedI have had this same problem. After searching for a solution, came across this post, found the function you mentioned and changed the value to 60 with no apparent side effects. Will update this post if I come across any problems.
If anyone can point me in the right direction as to if this function can be moved into the sites/modules or sites/theme folders and be rewritten as an override function, then that would be very much appreciated.
Comment #4
steinmb CreditAttribution: steinmb commentedI think there is no other way then to copy the entire code into your own custom menu module and maintain it there. I did not find a other way of solving it. I moved these functions.
Comment #5
kumkum29 CreditAttribution: kumkum29 commentedHello steinmb,
how do you implement these functions in your custom module? You modify the names?
Can we use theses functions in the template.php file of the admin theme?
Finaly, have you found a more simplest solution for alter the number of characters?
Thanks for your help.
Comment #6
hansrossel CreditAttribution: hansrossel commentedReally very annoying, 30 is too short, should be at least 60 or 72 by default.
Seems like Drupal 8 also has this limit:
https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Menu!MenuParentFo...
Comment #7
Sagar Ramgade CreditAttribution: Sagar Ramgade as a volunteer commentedIt seems 60 characters is the decent limit, Patch attached changes the value to 60.
Comment #17
Sivaji_Ganesh_Jojodae CreditAttribution: Sivaji_Ganesh_Jojodae as a volunteer and commentedRetest requested to see if the patch is still good.
Comment #18
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #19
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedVerified and tested by applying the patch.It was applied successfully.It looks good to me.Can be moved to RTBC.
Steps to test -
1. Go to the admin site.
2. Go to admin/structure/menu/manage/admin/add?destination=/drupal9.1.x-dev/admin/structure/menu.
3. Create a menu link.
4. Again Go to admin/structure/menu/manage/admin/add?destination=/drupal9.1.x-dev/admin/structure/menu.
5. Verify the parent link field.
Before Patch -
After Patch -
Comment #20
priyanka.sahni CreditAttribution: priyanka.sahni at Srijan | A Material+ Company for Drupal India Association commentedComment #22
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedApplied patch #7 and it works fine. I've added the word 'pneumonoultramicroscopicsilicovolcanoconiosis' as menu title and it comes normally as truncated in the parent link section.After applying this patch this problem is resolved.Adding screenshots below
before:
after patch:
Comment #23
Abhijith S CreditAttribution: Abhijith S as a volunteer and at Zyxware Technologies commentedComment #24
catchThe truncation was added in #151583: Menu - Fixes that make menu module work in D6 back in 2007. This was an omnibus patch fixing several issues in the menu system (while it was being rewritten), but where the 30 char truncation wasn't mentioned in the discussion.
Rather than increasing it to 60, I think we should just remove the truncation altogether.
Comment #25
raman.b CreditAttribution: raman.b at OpenSense Labs commentedRemoving the truncation from
parentSelectOptionsTreeWalk
Comment #26
steinmb CreditAttribution: steinmb as a volunteer commentedLooks right to me.
Comment #27
quietone CreditAttribution: quietone as a volunteer commentedSorry folks, this needs an IS update. It should include, at least, the proposed resolution. And up to date before and after screenshots will help as well.
I don't know but does this need a test?
Comment #28
raman.b CreditAttribution: raman.b at OpenSense Labs commentedApplying IS template and tagging for tests
Comment #29
Stockticker CreditAttribution: Stockticker at Internetdevels, Drupal Ukraine Community commentedRe-roll for a Drupal 8.9.x.
Comment #30
KapilV CreditAttribution: KapilV as a volunteer and at Innoraft for Drupal Care, Drupal Association commentedComment #32
Rinku Jacob 13 CreditAttribution: Rinku Jacob 13 at Zyxware Technologies commentedVerified and tested patch#30 on the drupal 9.3.x-dev version. Patch applied successfully and looks good to me.
Comment #34
vikashsoni CreditAttribution: vikashsoni as a volunteer and at Zyxware Technologies commentedApplied patch #30 applied in drupal-9.3.x-dev applied successfully
After patch the width of select container auto set according to text
Comment #36
yashingole CreditAttribution: yashingole at QED42 for Drupal India Association commentedComment #37
yashingole CreditAttribution: yashingole at QED42 for Drupal India Association commentedVerified and tested patch #30 on Drupal 9.5.x-dev. Patch applied successfully and looks good to me.
Testing steps:
1. Go to the admin site.
2. Go to admin/structure/menu/manage/main/add?destination=/d9.5/admin/structure/menu/manage/main.
3. Create a menu link.
4. Again Go to admin/structure/menu/manage/main/add?destination=/d9.5/admin/structure/menu/manage/main.
5. Verify the parent link field.
Testing Result:
1. After applying the patch the parent link is not truncated.
Can be move to RTBC
Comment #38
quietone CreditAttribution: quietone at PreviousNext commentedThis is a nice wee improvement!
Just a few things to do before this is ready for a committer.
Following the process in Triage the Drupal core RTBC queue to review this issue.
3. The issue title is a statement of the problem, probably better as the fix. 'Remove truncation from parent menu'
4. The issue summary is clear and mostly accurate. The remaining tasks says this this needs tests and this is tagged for tests. There are no tests in the patch nor any discussion of whether a test is needed or not. The screenshots are out of date, they are from #19 and the patch has changed since then and so has the default admin theme.
5. Needs a decision on testing in order to pass the Testing gate.
Based on the above points setting back to NW.
Duplicate sets of screenshots have been added for that same patch, removing credit for #22, #34.
The work here is suitable as a novice task, adding tag.
Comment #39
Rakhi Soni CreditAttribution: Rakhi Soni at Smashing Infolabs Pvt. Ltd. for Smashing Infolabs Pvt. Ltd. commentedAttached rerolled patch against drupal version 9.5x,,
Please review...
Comment #40
Prem Suthar CreditAttribution: Prem Suthar at Srijan | A Material+ Company for Drupal India Association commentedVerified and tested patch#30,Patch#39 and Patch#25 on the drupal 9.5.x-dev version. Patch applied successfully and looks good to me.We can move to RTBC.
Comment #41
Prem Suthar CreditAttribution: Prem Suthar at Srijan | A Material+ Company for Drupal India Association commentedVerified and tested patch#30,Patch#39 and Patch#25 on the drupal 9.5.x-dev version. Patch applied successfully and looks good to me.We can move to RTBC.
Comment #42
Manibharathi E R CreditAttribution: Manibharathi E R at Srijan | A Material+ Company for Srijan | A Material+ Company commentedPatch #39 Applied successfully on Drupal 9.5.x.
Comment #43
bnjmnm#30 still applies to 9.5, there's no need for the reroll in #39 and the issue should not be changed from "needs work" to "needs review" unless the work requested has been completed. Even if it was a necessary reroll, there's no need to review it as it's effectively the same already-reviewed patch.
Comment #44
joseph.olstadComment #45
joseph.olstadall that's left is to add a very simple parent item menu link title length longer than 30 characters assertion in the menu system tests for this
Comment #47
sahil.goyal CreditAttribution: sahil.goyal as a volunteer and at Srijan | A Material+ Company for Drupal India Association commentedPatch is does not have any major changes so it still applies for the version 10.1.x as being said there would not been any requirement to reroll the patch. even i tried to apply the patch it applied successfully and work as expected.
Comment #48
gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commentedComment #49
gaurav-mathur CreditAttribution: gaurav-mathur at Dotsquares Ltd. commentedHi apply the patch#39 on drupal 10.1.x-dev it applied successfully and work as expected. there would not been any requirement to reroll the patch.
adding screenshot for the reference.
We can move to RTBC.
Thank You
Comment #50
manojkumar_97 CreditAttribution: manojkumar_97 at OpenSense Labs commentedI have created a patch against #39 drupal version 10.1.x. We need to remove truncation from parent menu not truncation limit increment according the query.
pls review ...
Comment #51
manojkumar_97 CreditAttribution: manojkumar_97 at OpenSense Labs commentedComment #52
prasanth_kp CreditAttribution: prasanth_kp as a volunteer and at Zyxware Technologies commented#51 Patch Applied on 10.1.x-dev and work as expected.
Comment #53
manojkumar_97 CreditAttribution: manojkumar_97 at OpenSense Labs commentedComment #54
steinmb CreditAttribution: steinmb as a volunteer commentedIssue tagged with "needs tests" and I find no tests in #51.
Comment #55
jnlarAttached a patch adding a functional test. I've put the test in
MenuUiTest
since it seemed like the appropriate place. The test won't pass if you don't apply the patch from #51Please review!
Comment #56
jnlarFixed failed CSpell test
Comment #57
ameymudras CreditAttribution: ameymudras at Salsa Digital commentedTested the above patch on 10.1.x
1. The issue summary is clear and steps to reproduce have been provided with screenshots
2. Was able to reproduce the issue
3. The patch applies cleanly and fixes the issue with menu item truncation
4. Did a code review and no issues were identified
5. Tests have been provided
The before after screenshots have been provided above and not much has changed since so avoiding these again. Looks good for RTBC
Comment #58
quietone CreditAttribution: quietone at PreviousNext commentedI read the Issue Summary. I do not see any difference between the before and after screen shots. I am missing something. If so, perhaps a comment to help the reviewer.
I applied that patch and tested by making an extremely long menu link. The result is not very friendly. I do like that in ends in an ellipsis. See the attached screenshot. Is removing truncation the best option here? I now think this needs a usability review. Adding tag.
This test is only adding a link so it can be added to an existing test. That will allow us to not increase testing time and cost. I did not look closely but \Drupal\Tests\menu_ui\Functional\MenuUiTest::addMenuLink looks like a possible choice.
Comment #59
bnjmnmThe before and after in the issue summary were referencing the same image. I updated the "after" to reference the image intended to be shown there. Note that the screenshot in #58 shows that there may be other "after" scenarios to consider beyond the ideal-scenario one presented in the IS.