Posted by wesku on November 3, 2011 at 6:38pm
5 followers
| Project: | Big Menu |
| Version: | 6.x-1.0 |
| Component: | Miscellaneous |
| Category: | feature request |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
The same problem exists for D7 core. There is one existing solution in D.o as far as I know: http://drupal.org/project/menuadminsplitter
It's not exactly the same thing, but at least it takes care of the same problem on Drupal 7.
Comments
#1
updating issue title.
#2
Anyone had a go at this? In the next week I need to port this module for a client and it would be great to compare what I've got so far to anyone else's attempts.
Cheers
#3
I've not got anything in the pipeline for this.
The menu admin screen has probably been significantly rewritten in D7, so it'll probably be much of a rewrite to integrate this approach with it. Possible, but heavily different.
#4
I just checked, and the current 6-dev is clean according to coder review and grammar parser, so it's a good candidate for branching to D7 today.
(I make it a habit to run the syntax reformatter over a d6 branch before branching for further dev)
Have at it!
You may find a few corners (such as the error handling when AJAX starts failing) that just never worked, but this has been in production on two huge sites for 6 months with no complaints, so it should be good to go.
#5
Awesome cheers, I might post my progress on here early next week
#6
Here's the work in progress, just enable and clear cache and you should roughly see what's going on. Currently need to figure out why the ajax for the menu children is getting a parse error and why all the expanded tickboxes are being rendered outside the table.
#7
Just realised the expanded tickboxes shouldn't actually be there -_-
#8
It's a solid start.
Things I've noted in a visual review & test
The parse error may be related to an earlier work-around related to the jquery version. I'll see if I can spot it.
There is now a jquery once() function that could be used to replace our .addClass('bigmenu-processed')
To debug, you can : right-click the 'expand' link (This opens a sorta-broken submenu page) - then add '/js' to the URL. Inspect that JSON, which is where the error is found.
I did so and found our JSON was also triggering some (syntax error-style) HTML tagged on the end of the return valus.
I found
Error message
So.
A: fix the code complaint.
B: See if the JSON is returning immediately as is expected.
#9
If we are going to make progress, then zip files won't do it :-)
I've added you as a maintainer and given you a D7 7.x-1.x branch (Containing the zipped updates you attached above) you can re-fetch from git.
See http://drupal.org/node/1104872/git-instructions/7.x-1.x for the details.
Feel free to tweak, commit, push as much as is needed until it starts working. The D7 branch doesn't work yet, so anything you throw at it will be an improvement,
Let me know if we want to review for an actual tagged release though.
Cheers!
#10
Awesome thanks a lot, I'll be solidly working on this over the next couple of days, hoping to get it working by Friday.
#11
note re. my error paste above. I'm only seeing all that because I'm running 5.3 with bitchy warnings on.
If your error log level is lower, you won't get them all, but I recommend turning up the error volume and writing PHP-strict compliant code. It helps in the long run.
#12
Just committed a semi-working version, show children is now working correctly but reparenting and saving doens't actually update the menu item's parent. Changing weight, disabling, etc works though.
#13
This is something to do with the fact that the form object doesn't know about the new menu items that are in the form_state['input'] array so when the submit handler merges the $form_state['input'] into the form the menu items become single value items instead of the full menu item. This is apparent because reparenting a top level menu item still works because the menu items haven't been dynamically pulled in.
#14
Hurray! Fixed that problem with latest commit :)
#15
O_o um. Yeah, that sounds like the sort of explanation I'd have to come up with while deep in the code.
NVM, if you identified it that's certainly a good step.
I'll see what the latest version does for me!
#16
Hey groovy we are working!!
You even managed to get the row weights to go away on the loaded trees - something I didn't quite sort out last time.
Seems to be adding the "show row weights" text at the top repeatedly due to it - but I know why it's doing that. Related to the reason I didn't follow through with it in the first place, although this D7 sortable library seems less destructive than before.
Small issue - and it may be inherited from the original even - If I move things in an expanded menu and then close it again before saving, the changes are lost.
"Hide Children" Is *removing* it from the DOM? Can we just hide it and let the form submission continue to work?
However, it's basically as workable as it was before. Very near a release I'd say! Good job.
#17
Yeah, I also want to get it so if you have a parent menu item expanded AND one of it's children, if you hide the parent it should hide the children's expanded menu too. But yes, removing them from the DOM probably isn't the smartest thing to do, maybe display:none; would work better?
#18
Latest commit now means that hiding elements won't remove them from the DOM, and showing them again will just un-hide them!
#19
IIRC, last time I tried (something like) that - I had the same thoughts - but then trying to 'hide' table-level elements (eg, the tr) made my browsers go screwy.
If you've cracked it, that's great news.
Very sexy. Yes, behavior there is now as expected.
You want to tag it with a point release when you are happy? I'm feeling good about it.
Would be nice to sort out the doubled-up messages at the top before calling it stable, but either way, it does all the job that it is supposed to.
#20
I'm not sure what messages you are talking about being doubled up? The thing I'm seeing that needs to be addressed is the mountain of these watchdog messages:
Notice: Use of undefined constant DRUPAL_WEIGHT_SELECT_MAX - assumed 'DRUPAL_WEIGHT_SELECT_MAX' in form_process_weight() (line 3801 of /home/adamb/projects/mohpub/includes/form.inc).It's strange because this constant isn't actually defined anywhere, it used to be in system.module from what I can see. What's even more strange is the fact that the menu items are almost rendered exactly the same as the normal menu overview page, especially the weight element which is where this error comes from.
#21
Also, I can't see a use for the bigmenu_alias_autocomplete() function or the menu callback that uses it. It still contains the same code from the D6 version, I'm assuming it's not used and can be removed?
#22
bigmenu_alias_autocomplete was used by the bigmenu addition to the node edit form.
When you have thousands of menu items, each node edit form ends up with an unusable select box with each item in.
So I replaced the huge select box with an autocomplete.
I see that code has been pulled out already, so with that gone, yes, the autocomplete callback is now redundant.
http://drupalcode.org/project/bigmenu.git/blob/refs/heads/6.x-1.x:/bigme...
See bigmenu_form_alter
The messages at the top are the ones inserted by script each time. After playing a bit, quite a few of them collect.
pic attached
#23
That's strange, I don't get the duplicate messages at all :/ and the "show row weights" appears as an icon. What version of core are you running?
#24
Ok yeah I see where the duplicate messages are coming from. Any ideas on how to fix that?
#25
Committed fix that hides the tabledrag js warning once children have been generated.
#26
yep, the tabledrag is going away. Still seeing show row weights. Not sure who is providing that.
My core is 7.13-dev (last week) but totally vanilla. (ONLY this module + devel added)
If you're not seeing it, I wouldn't worry.
#27
When I did d6, I started getting those too, but at the time couldn't attack the tabledrag.js enough to work around it. Good that you found a way.
#28
hah - you extended the tabldrag object to kill its function!
Drupal.theme.tableDragChangedWarning
OK, that works..
API like that wasn't there in D6 when I looked - was a monolithic func
#29
the message I see is from Drupal.tableDrag.prototype.hideColumns
- it really isn't expecting to be called more than one time per page - that's our cheat.
no simple fix without copying and replacing the full func that I can see
#30
Closing as 7.x-1.x-dev branch has been released! Cheers for your help dman
#31
Booyah!