Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
HI there,
I have added a minor feature to site map.
I have added the ability to order the menus that are displayed in site map, would you "the maintainers" like a copy of this code ??
Since I altered open source code, I'd like to give it back ..... so here it is.
I didn't go for the usual "javascript drag to order option" as I felt it was probably only going to get minimal use and a more traditional approach would suit.
Hope this helps..... didn't know how to do this "correctly" so if this is not proper channel, please forgive me.
regards
Steve
Comment | File | Size | Author |
---|---|---|---|
#62 | site_map-allow_ordering_content-1529708-61.patch | 15.67 KB | ron_s |
#56 | site_map-allow_ordering_content-1529708-56.patch | 15.07 KB | ron_s |
#48 | allow_ordering_of_site-1529708-48.patch | 13.89 KB | helmo |
#41 | site_map_ordering-custom-patch-2.patch | 14.49 KB | janekD7 |
#40 | site_map_ordering-custom-patch-1.patch | 14.45 KB | janekD7 |
Comments
Comment #1
imperial_dalek CreditAttribution: imperial_dalek commentedIn my excitement I forgot to tidy code and remove activity log comments used during development.
So here attached version that does not output comments to activity log.
I hope someone finds ability to sort / order site map menus useful.
regards
Steve
Comment #2
frjo CreditAttribution: frjo commentedThanks for contributing to Site map!
Please, if possible, provide your changes as a patch against the 7.x-1.x branch.
Documentation about patches http://drupal.org/patch
Comment #3
imperial_dalek CreditAttribution: imperial_dalek commentedno problem, I will be happy to do that.
However I have never created a patch before
Please describe how to do that.
regards
Steve
Comment #4
frjo CreditAttribution: frjo commentedThe best documentation about how to work with patches are at http://drupal.org/patch.
Comment #5
TarKHaoS CreditAttribution: TarKHaoS commentedThis is ABSOLUTELY useful.
I've tried it and I aprove it.
As suggested you should try to make a clean patch for that.
Thanks again ^^
Comment #6
darrell_ulm CreditAttribution: darrell_ulm commentedHey this is nice, anyone with some time want to roll the patch file? I want to put this in, as it is a cool option, but will not have time in the next few weeks.
Thanks
Comment #7
torpy CreditAttribution: torpy commentedDid some major cleanup of the code, comments and UI. Should be easier to understand now. Also reduced some of the complexity of the ordering code. Patch attached against 1.x-dev.
It would be great if this had drag-and-drop ordering but I don't have the time to implement that right now.
Comment #8
darrell_ulm CreditAttribution: darrell_ulm commented@torpy and others, thanks for the patch, will give it a check.
Thanks again!
Comment #9
darrell_ulm CreditAttribution: darrell_ulm commentedTorpy,
The patch applied fine, and the form items to order the items worked in the form (although I had 6 options and the count only went from 1-5), but in the output the numbers did not change the ordering. I tried several ways and cleared the cache for each test.
So I couldn't make it sort, although it did apply. Can you take another look at it?
Thank you,
Comment #10
torpy CreditAttribution: torpy commentedOuch, that's a pretty big issue, completely my fault: I was, for some unfathomable reason, using asort() instead of ksort(). Fixed that issue and the "n-1" options issue as well.
This patch should sort things out :)
Comment #11
torpy CreditAttribution: torpy commentedComment #12
darrell_ulm CreditAttribution: darrell_ulm commentedNice, thanks for the fast update.
Hey, I just was checking out drag and drop sorting on the weekend and had a test case working, so after we get yours in there, I'm interested in adding that for the interface.
Comment #13
torpy CreditAttribution: torpy commentedSounds great, if you need a hand just let me know!
Comment #14
darrell_ulm CreditAttribution: darrell_ulm commentedTorpy,
Yep, applies and works and provides the functionality. It would be cool if this could be folded into the menu selection area (Menus to include in the site map) with drag and drop reordering. ( http://api.drupal.org/api/drupal/includes!common.inc/function/drupal_add... )
Then again, it works. This is a @frjo question since this module is his.
@frjo what do you think? Should we integrate this to sort the menus or do some fancy ordering to make the interface cleaner?
Thanks!
Comment #15
DuaelFrGood for me.
UI could be better but this is another point.
Comment #16
DuaelFrOops ! I am sorry I forgot I had to reroll the patch ;)
Here it is. Turning back to "need review".
Comment #17
markwittens CreditAttribution: markwittens commentedI was looking for something like this. Patch applies like it should and it works great, thanks for the addition!
Comment #18
new123456789 CreditAttribution: new123456789 commentedHow would I go about applying this patch?
Comment #19
darrell_ulm CreditAttribution: darrell_ulm commentedhttp://drupal.org/patch/apply
Comment #20
dags CreditAttribution: dags commentedI could only apply the patch in #16 by updating to latest dev branch and using
patch -p1 < site_map-menu_ordering-15129708-16.patch
. But it does work.Comment #21
darrell_ulm CreditAttribution: darrell_ulm commentedYes, sounds like it applied. I'll take another look unless anyone else wants to grab it.
Thanks people!
Comment #22
BWPanda CreditAttribution: BWPanda commentedHere's a brand new patch that allows all site map content to be ordered! It:
I've attached a screenshot of my site map settings form showing the new ordering table and the way I now have my site map ordered (Front page, Main menu, Topics (vocab), Packages (vocab), Footer menu).
As the patch is fairly long and involved, I'll summarise what each hunk does:
Give it a test and let me know how you go. Hopefully we can get this committed soon!
Comment #23
Jarbrain CreditAttribution: Jarbrain commentedHello everyone,
Great module by the way! Could someone do me a favor by applying the latest patch to the 'site_map.tar_.gz' and send it to me as I am unable to apply any patches due to my host provider not allowing me access to SSH or run GIT, etc.
Cheers,
Jarbrain
Comment #24
BWPanda CreditAttribution: BWPanda commentedYou should just be able to do this yourself on your local machine (download latest version, apply patch, zip up results and upload to server)...
Comment #25
Gerben Zaagsma CreditAttribution: Gerben Zaagsma commentedThe patch works fine for me. Thanks @BWPanda !
Comment #26
dman CreditAttribution: dman commentedPatch applies well - it's a great addition I've had to do dirty hacks to get around before.
Comment #27
acarpio CreditAttribution: acarpio commentedPatch is working for me. But I'm getting these notices on the site-map admin page:
Notice: Undefined index: front en site_map_admin_settings_form() (línea 135 de /Library/WebServer/Documents/todo/comer/sites/all/modules/site_map/site_map.admin.inc).
Notice: Undefined index: front en site_map_admin_settings_form() (línea 138 de /Library/WebServer/Documents/todo/comer/sites/all/modules/site_map/site_map.admin.inc).
Thanks for the patch!
Comment #28
acarpio CreditAttribution: acarpio commentedI guess that the notices I mentioned on the site-map admin page are due to something on my side. I'll try to figure out and will report it if I find the problem.
Comment #29
acarpio CreditAttribution: acarpio commentedWell I don't have Notices on site-map admin page anymore and I didn't do anything. I guess it was cache related. Sorry for my "spam".
Comment #30
garethhallnz CreditAttribution: garethhallnz commentedTested #22 works great Thank you.
Comment #31
annikaC CreditAttribution: annikaC commented+1 on #22, works great!
Comment #32
darrell_ulm CreditAttribution: darrell_ulm commentedApologies that I haven't had time to commit this as I'm off working on other projects
Is it possible to get a final review from a site_map maintainer to commit to dev?
Kudos! Looks like a nice improvement!
Comment #33
jdanthinne CreditAttribution: jdanthinne commentedPatch is working fine (ordering the menus), but it needs an upgrade function. After applying the patch, the site map page is empty, until you go to its settings page (with tons a notices) and save the settings, then the page is filled again, and notices are gone.
Comment #34
darrell_ulm CreditAttribution: darrell_ulm commentedNot maintaining this anymore, but looks like the patch needs just a little more work. Ordering is a nice feature, if the original patch author or anyone else of course needs the patch and has time to re-roll and fix, it would be great!
Thanks for the update @jdanthinne.
Comment #35
omerida CreditAttribution: omerida commentedI've uploaded a new patch for the 7.x-1.2 release.
Comment #36
john.oltman CreditAttribution: john.oltman commentedTried to apply this but site map giving errors after the update.
Comment #37
helmo CreditAttribution: helmo commented@john.oltman: Could you be more specific? What is the error? a screenshot may help?
Comment #38
ishworthapaliya CreditAttribution: ishworthapaliya commentedSimilar problem as #36, I get erros after applying the patch #35. Attaching screenshot of the errors with this message.
Thanks!
Comment #39
Anonymous (not verified) CreditAttribution: Anonymous commentedI had the same errors.
Hmmm special... i was looking for the problem and had the line 125 in the file site_map.admin.inc debugged. I printed the variable $content_id with dpm() and now, the error is away! :D Maybe it was a caching problem, the patch #35 is working now.
Comment #40
janekD7 CreditAttribution: janekD7 commentedI fixed above issues, here is the patch. This is a standalone patch you don't need to apply any other patches from this thread. It is working for site_map version 1.2
Unfortunately it's just quickfix for my project and I'm not sure if it is following all Drupal conventions related to patches (if there are any).
Erorros mentioned in #38 was caused by missing .js file and undefined initial values for sort rows. What is more I also fixed the issue with taxomony terms (they were always displayed in draggable table because of naming mistake).
Comment #41
janekD7 CreditAttribution: janekD7 commentedFixed one more issue discovered in previous patch connected with taxonomy terms loading by id instead of loading by name.
Comment #42
darrell_ulm CreditAttribution: darrell_ulm commentedAnyone test this in the field yet? Thank you!
Comment #43
fubarhouse CreditAttribution: fubarhouse commentedI'm also keen on getting this one tested and released, what can I do to help?
Comment #44
john.oltman CreditAttribution: john.oltman commentedIf you want to help, apply the patch #41 on your local install and report back if it worked. If it worked, then I can commit.
Comment #45
darrell_ulm CreditAttribution: darrell_ulm commentedTested: patch in #41 applied fine and tested a-ok. No errors in the log, and sorting worked. Like it, nice patch!
Should I integrate into dev or is someone else currently doing that. Thank you.
Comment #46
john.oltman CreditAttribution: john.oltman commentedI'll commit to DEV within the next few hours.
Comment #47
john.oltman CreditAttribution: john.oltman commentedReceived these errors when using git apply -v for patch #41 against the tip of 7.x-1.x:
error: patch failed: site_map.module:436
error: site_map.module: patch does not apply
Was the patch generated using a git diff and the 7.x-1.x-dev branch (which is 4 commits ahead of 7.x-1.2)?
Comment #48
helmo CreditAttribution: helmo commentedIt seems that the patch was indeed against 7.x-1.2. The conflict was caused by:
On closer inspection I noticed that the hook drupal_alter call is gone ...
Not sure if that was intended. Here's a re-roll of the patch against 7.x-1.x.
Comment #50
john.oltman CreditAttribution: john.oltman commentedCommitted and pushed #48. Please download the 7.x-1.x DEV branch after it builds and verify. I put the drupal_alter back in.
Comment #51
artbussy CreditAttribution: artbussy commentedI downloaded the 7.x-1.x DEV branch twice. It seems the js-file is missing. After copying the js from the patch and uploading, ordering of the sitemap works.
Comment #52
helmo CreditAttribution: helmo commented@john.oltman: @artbussy seems to be right..
Comment #53
ron_s CreditAttribution: ron_s commentedThe issue is most likely because the patch in #48 does not apply cleanly. I first tried to apply it to 7.x-1.2, and I get an error message about a failed attempt to patch site_map.module for the _site_map_menus function (around line 443).
I then downloaded 7.x-1.x-dev per #50, and as mentioned by @artbussy and @helmo, the site_map.admin.js file is not included. It is necessary to create the JavaScript file using the code that's in patch #48. Once this file is in in the site_map module directory, the 7.x-1.x-dev version works.
Comment #55
colanKicked out the previous commit. Let's wait until there's an associated JS file that works before putting this in.
Comment #56
ron_s CreditAttribution: ron_s commentedHere's an updated patch that includes the JavaScript file. There's one addition in the newest 7.x-1.x-dev
_site_map_menus
function that wasn't there before:I moved this to the
template_preprocess_site_map
functionswitch
statement, since that's where thevariable_get('site_map_show_menus', array());
now exists.Comment #58
ron_s CreditAttribution: ron_s commentedNot sure why the test bot is returning "Output: [error: site_map.admin.js: No such file or directory]."
I applied the patch locally without issue. Let me know if anyone has problems.
Comment #60
dman CreditAttribution: dman as a volunteer commentedThe patch seems to by trying to apply a *change* to site_map.admin.js - which is not present in 7.x-1.x
http://cgit.drupalcode.org/site_map/tree/?h=7.x-1.x
I *think* that you need to add -N to the args when creating the patch.
I would have expected the line where it goes
to have been
or something.
But that's just an observation. I'm not suggesting editing it by hand.
Comment #61
ron_s CreditAttribution: ron_s commentedThat's odd, since I did "add -N" before creating the patch. Hmm... I'll try again and see if I can get it to build correctly.
Comment #62
ron_s CreditAttribution: ron_s commentedCould not get
add -N
to show/dev/null
, so I usedformat-patch
instead. This worked.Comment #63
ron_s CreditAttribution: ron_s commented@colan, I see that 7.x-1.3 was just released. This patch should be good to go... I fixed the associated javascript file a couple of months ago.
I see in the release notes that it was added and then removed... I'm assuming that was from the previous problems with the JS file. Is there anything that we need to do further with this? Thanks.
Comment #64
colanSomeone else needs to successfully test it. When it gets to RTBC, I'll commit it.
Comment #65
danieltome CreditAttribution: danieltome commentedI've applied the patch site_map-allow_ordering_content-1529708-61.patch (I had to manually set the base path, in order to get the patch working, but it worked fine and ordering looks good)
Let me know if you want me to submit any screenshots, or anything else.
Comment #66
ron_s CreditAttribution: ron_s commented@danieltome, the patch includes a new file (site_map.admin.js). This is normal behavior. Thanks for the review.
Comment #67
tancPatch in #62 applies cleanly and ordering is working.
Comment #68
remaye CreditAttribution: remaye commentedSame for me. Thanks for the patch.
Comment #69
ron_s CreditAttribution: ron_s commentedThanks, anyone want to mark as RTBC?
Comment #70
tancComment #80
Nafes CreditAttribution: Nafes as a volunteer commentedСommitted. Thanks!
Comment #81
DuaelFr\o/
Thanks :)
Comment #82
Nafes CreditAttribution: Nafes as a volunteer commented:-)
Comment #84
darrell_ulm CreditAttribution: darrell_ulm as a volunteer commentedI don't believe a new release has been made in a while for this version with the ordering, can there be a 7.x-1.4 release with these changes or more like a parallel stable release with ordering?
Comment #85
darrell_ulm CreditAttribution: darrell_ulm as a volunteer commentedTesting this, one potential issue may be when a site has too many resources to sort. Making an issue.
Comment #86
Vali Hutchison CreditAttribution: Vali Hutchison commentedInstalled the dev version of site map module today which has this ordering update included and works very well. Be great to see this in the main branch.