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

Comments

imperial_dalek’s picture

StatusFileSize
new17.54 KB

In 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

frjo’s picture

Version:7.x-1.0» 7.x-1.x-dev

Thanks 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

imperial_dalek’s picture

no problem, I will be happy to do that.

However I have never created a patch before

Please describe how to do that.

regards

Steve

frjo’s picture

The best documentation about how to work with patches are at http://drupal.org/patch.

TarKHaoS’s picture

This 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 ^^

quantumtime’s picture

Status:Active» Needs work

Hey 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

torpy’s picture

Title:Update to Site Map» Allow menu ordering
Category:task» feature
Status:Needs work» Needs review
StatusFileSize
new5.01 KB

Did 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.

quantumtime’s picture

@torpy and others, thanks for the patch, will give it a check.

Thanks again!

quantumtime’s picture

Status:Needs review» Needs work

Torpy,

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,

torpy’s picture

StatusFileSize
new4.88 KB

Ouch, 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 :)

torpy’s picture

Status:Needs work» Needs review
quantumtime’s picture

Nice, 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.

torpy’s picture

Sounds great, if you need a hand just let me know!

quantumtime’s picture

Torpy,

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!

DuaelFr’s picture

Status:Needs review» Reviewed & tested by the community

Good for me.
UI could be better but this is another point.

DuaelFr’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new4.97 KB

Oops ! I am sorry I forgot I had to reroll the patch ;)

Here it is. Turning back to "need review".

markwittens’s picture

I was looking for something like this. Patch applies like it should and it works great, thanks for the addition!

new123456789’s picture

How would I go about applying this patch?

quantumtime’s picture

dags’s picture

Status:Needs review» Reviewed & tested by the community

I 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.

quantumtime’s picture

Assigned:Unassigned» quantumtime

Yes, sounds like it applied. I'll take another look unless anyone else wants to grab it.

Thanks people!

BWPanda’s picture

Title:Allow menu ordering» Allow ordering of site map content
Assigned:quantumtime» Unassigned
Status:Reviewed & tested by the community» Needs review
StatusFileSize
new13.31 KB
new47.71 KB

Here's a brand new patch that allows all site map content to be ordered! It:

  • Provides ordering via a drag-&-drap table (copied the functionality from the core Filter module)
  • Allows all content to be ordered (i.e., not just menus)
  • Allows you to separate content (i.e., you don't have to have all your menus grouped together; you can have one menu first, followed by some vocabularies, then another menu at the bottom)

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:

  • site-map.tpl.php - Changes separate output of site map content into a single variable (so it can be ordered)
  • site_map.admin.inc - Creates an array of content that it then loops through to populate the draggable table
  • site_map.admin.js - New file that makes the draggable table only display enabled content (copied from Filter module)
  • site_map.module - Adds new theme function for draggable table, alters menu and vocabulary functions to allow outputting one menu/vocab at a time (instead of grouping them all together)
  • site_map.theme.inc - Orders site map content and outputs it all as one variable for the template file, defines new theme function for draggable table

Give it a test and let me know how you go. Hopefully we can get this committed soon!

Jarbrain’s picture

Hello 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

BWPanda’s picture

You should just be able to do this yourself on your local machine (download latest version, apply patch, zip up results and upload to server)...

Gerben Zaagsma’s picture

The patch works fine for me. Thanks @BWPanda !

dman’s picture

Status:Needs review» Reviewed & tested by the community

Patch applies well - it's a great addition I've had to do dirty hacks to get around before.

acarpio’s picture

Patch 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!

acarpio’s picture

I 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.

acarpio’s picture

Well 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".

garethhallnz’s picture

Tested #22 works great Thank you.

annikaC’s picture

+1 on #22, works great!

quantumtime’s picture

Apologies 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!

jdanthinne’s picture

Patch 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.

quantumtime’s picture

Not 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.

omerida’s picture

Issue summary:View changes
StatusFileSize
new12.42 KB

I've uploaded a new patch for the 7.x-1.2 release.

john.oltman’s picture

Status:Reviewed & tested by the community» Needs work

Tried to apply this but site map giving errors after the update.

helmo’s picture

@john.oltman: Could you be more specific? What is the error? a screenshot may help?

ishworthapaliya’s picture

StatusFileSize
new92.5 KB

Similar problem as #36, I get erros after applying the patch #35. Attaching screenshot of the errors with this message.

Thanks!

FeanorCC’s picture

I 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.

janekD7’s picture

StatusFileSize
new14.45 KB

I 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).

janekD7’s picture

StatusFileSize
new14.49 KB

Fixed one more issue discovered in previous patch connected with taxonomy terms loading by id instead of loading by name.

quantumtime’s picture

Status:Needs work» Needs review

Anyone test this in the field yet? Thank you!

fubarhouse’s picture

I'm also keen on getting this one tested and released, what can I do to help?

john.oltman’s picture

If 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.

quantumtime’s picture

Status:Needs review» Reviewed & tested by the community

Tested: 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.

john.oltman’s picture

I'll commit to DEV within the next few hours.

john.oltman’s picture

Received 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)?

helmo’s picture

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new13.89 KB

It seems that the patch was indeed against 7.x-1.2. The conflict was caused by:

commit 9506e4a6267c0fbd1fc4da90554753d3d30d38aa
Author: Amit Goyal <mintoo@gmail.com>
Date:   Tue May 13 18:41:05 2014 +0530

    minor coding standard issues fixed in site_map.module

On closer inspection I noticed that the hook drupal_alter call is gone ...

<?php
// Add an alter hook so that other modules can manipulate the
// menu tree prior to rendering.

$alter_mid = preg_replace('/[^a-z0-9_]+/', '_', $mid);
drupal_alter(array('site_map_menu_tree', 'site_map_menu_tree_' . $alter_mid), $tree, $menu);
?>

Not sure if that was intended. Here's a re-roll of the patch against 7.x-1.x.

  • 7124eb7 committed on 7.x-1.x
    Issue #1529708 by imperial_dalek, torpy, janekD7, helmo: Allow ordering...
john.oltman’s picture

Committed and pushed #48. Please download the 7.x-1.x DEV branch after it builds and verify. I put the drupal_alter back in.

artbussy’s picture

I 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.

helmo’s picture

Priority:Normal» Major
Status:Needs review» Needs work

@john.oltman: @artbussy seems to be right..

ron_s’s picture

The 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.