Hello,
I want to start by saying I really like this module and thank you for creating it. Hopefully my request isn't too much trouble.
Since the site map page does not output a heading1 page title (which is not a problem), I have to add that in the message area provided on the admin page. Putting a heading within a paragraph is not valid. I could also see wanting to put all kinds of other things in that area like multiple paragraphs, lists, images, or divs and none of these should be wrapped in paragraph tags. It would be perfect if it were just wrapped in the div and everything inside was added in the input field in the admin area.
So I would like to suggest removing the paragraph tags from the output of the site map message. I believe it is the first function in the module file. I am not a programmer so I don't know how to create a patch but I assume the code would look something like this.
/**
* Implementation of hook_help().
*/
function site_map_help($section) {
switch ($section) {
case 'sitemap':
$output = _sitemap_get_message();
return $output ? $output : '';
}
}
Thanks
Comment | File | Size | Author |
---|---|---|---|
#28 | site_map_247077_28.patch | 17.44 KB | frjo |
#24 | site_map_247077_24.patch | 15.36 KB | frjo |
#23 | site_map_247077-23.patch | 12.65 KB | tic2000 |
#21 | site_map_247077_21.patch | 10.25 KB | frjo |
#11 | site_map_247077-11.patch | 6.02 KB | tic2000 |
Comments
Comment #1
hass CreditAttribution: hass commentedSeems no more a case... page have a heading and sitemap message is correctly inside a paragraph - or I do not understand anything.
Comment #2
eff_shaped CreditAttribution: eff_shaped commentedIs this really fixed?
I'm interested in altering the position of site map message - in D6. Trying to access the message output in my theme. Is this possible?
It seems that using hook_help() to add a paragraph to the page makes the output less themeable.
It's not really help text is it? More of a description...
My theme inserts the help text above the breadcrumb; which is not where I want it.
I've used a hard coded text solution in my template.php
I'd much rather use the site map message from the module settings page - more usable for site editors.
This is the code in D6 sitemap.module:
This is a fine module - I want to keep using it and for it to look good also.
Comment #3
hass CreditAttribution: hass commentedThe hook_help is not for your output. Take a look - I'm not sure if there is a site_map.tpl.php available.
Comment #4
eff_shaped CreditAttribution: eff_shaped commentedA site_map.tpl.php would be awesome. I wish there was one. *sigh*
Comment #5
frjo CreditAttribution: frjo commentedUsing hook_help to output "site_map_message" is an old leftover. Now that you brought it up I think it makes more sense to move it to theme_site_map_display().
What do you think?
A template_preprocess_site_map() plus a site_map.tpl.php is an interesting idea. If anyone knows how to do it please submit a patch. Otherwise I will take a look at it sooner or later.
Comment #6
hass CreditAttribution: hass commentedComment #7
hass CreditAttribution: hass commentedSitemap is more like a node... and someone else asked for a sitemap in a block... http://drupal.org/node/358426
Comment #8
eff_shaped CreditAttribution: eff_shaped commented@frjo - Putting the message into a themeable place would be a superb development.
Since I'm using the theme_site_map_display() in my theme template.php, it would work well for me.
And it would certainly add to themabilty to have that tpl file.
Comment #9
eff_shaped CreditAttribution: eff_shaped commentedsorry, somehow I changed the title back in error. Correcting that.
Comment #10
beyond67 CreditAttribution: beyond67 commentedHas any progress been made on themeing the sitemap? I would like to output the tags in multiple columns, instead a single long column.
Comment #11
tic2000 CreditAttribution: tic2000 commentedHere is a patch. What it does?
1. On the settings page adds an input format to the text area used for the message.
2. Outputs the message using check_markup().
3. Moves the template for the final output to a file instead of a function.
4. Wraps each of the sections in a div and adds a class to them. Of course those can be changed in the template file.
5. Moves the commentrss in its own theme function.
1 and 2 will help solve the problem OP had.
3-5 adds the site-map.tpl.php file.
Right now only for the page. If this is reviewed and hopefully committed I will create a patch to make the block a template file too if it will still be desired.
The patch is for the 2.x-dev version which should be the one to add new features to.
Comment #12
tic2000 CreditAttribution: tic2000 commentedwrong status
Comment #13
hass CreditAttribution: hass commentedLooks promissing... There is only one bug at "additional" where you missed to remove a dot ".="
I can only speak codewise... Untested. I will try to test asap. Really hope to get this cleanup in...
Comment #14
tic2000 CreditAttribution: tic2000 commentedI did not miss. It's in a foreach loop so it has to add to the string each loop.
Comment #15
hass CreditAttribution: hass commentedUps, ok... my mobile phone display was a bit small for a review :-)
Comment #16
hass CreditAttribution: hass commentedMarked #730992: Options for title of sitemap page as duplicate
Comment #17
hass CreditAttribution: hass commentedMarked #747032: use theme functions as duplicate
Comment #18
tic2000 CreditAttribution: tic2000 commented@hass
Please stop doing that.
None of those 2 issues are duplicates. This issue should have been split in 3 smaller one.
1. Don't output the message in paragraph tags.
2. Use a template file for the final output.
3. Use a theme function for commentrss output.
Not any theme issue is in the scope of this one and won't be solved by this one.
This is how issues should be handled and are handled at least for core. One issue at a time in small patches easy to review and commit and easy to roll back in case of a problem.
Comment #19
hass CreditAttribution: hass commentedIt is. You may have not understood theming or what the people are asking for
Comment #20
tic2000 CreditAttribution: tic2000 commentedIf they don't ask for a template file for the final output or removing the p tags around the message then it's not a duplicate.
Like I said not all theme issues will be solved by this one.
Comment #21
frjo CreditAttribution: frjo commentedA nice patch tic2000, this I will commit after we resolved some minor issues.
I have attached a revised patch. Moving all theme functions to site_map.theme.inc, rename theme_site_map_commentrss to theme_site_map_rss_legend and other minor stuff.
My main concern is that we know end up with HTML code like this:
That is one div to much I think. One is from site-map.tpl.php and the other from theme_site_map_box().
Any good suggestions?
Comment #22
tic2000 CreditAttribution: tic2000 commentedI think it's good practice to wrap all the output in a div with a class for easier css theming. Otherwise, for garland for example, everything will be wrapped in a div with class clear-block.
Front page has only one div sitemap-box inside and the wrapper may seem to much, but the menus and taxonomies may have more and that's where a wrapper is useful.
But in the end, whatever is fine. The template file gives the option for everyone to add or remove that div if they so desire. So it's your decision.
Your patch is missing site-map.tpl.php.
If you did create the theme file I think you should move the preprocess function there too.
For the missing file I set to cnw
Comment #23
tic2000 CreditAttribution: tic2000 commentedPatch to solve the problem and implement the suggestion in #22.
I also made some small changes in phpdoc section of the files to conform to drupal standards. (empty line between @file and the short description).
Again, if you feel like removing those wrapper divs feel free to do so. I don't think it's an issue worth holding this back and it can be easy modified by the user.
Comment #24
frjo CreditAttribution: frjo commentedReworked the class names a bit, what do you think?
I believe we have something to commit know, thanks for contributing this!
Do you believe I should make a 2.0 release and make it the recommended version?
Comment #25
tic2000 CreditAttribution: tic2000 commentedI see no critical issue open for 6.x-2.x-dev.
Unless someone has any critical issue I think a 2.0 release could be done.
IMO the patch is RTBC, but I did contributed to it and I can't set it as RTBC.
Comment #26
tic2000 CreditAttribution: tic2000 commentedYou should have consistency with the class names. Use "site-map" everywhere.
Powered by Dreditor.
Comment #27
hass CreditAttribution: hass commentedremoved
Comment #28
frjo CreditAttribution: frjo commentedHopefully the last patch. Fixed consistency for the class names.
Comment #29
tic2000 CreditAttribution: tic2000 commentedYes. Enough scope creep in this issue.
My comment was only about that piece of code. I didn't even see you have "sitemap" in a lot of other places. It should have been another issue opened for that.
I think this is ready to fly.
Comment #30
frjo CreditAttribution: frjo commentedCommitted to 6-2-dev. Will make a release shortly.
Comment #31
hass CreditAttribution: hass commentedHow about blocks template?
Comment #32
frjo CreditAttribution: frjo commentedTo add a template for the current site map block is unnecessary I believe, the block is very simple and small.
A feature request to add a new block that contains the site map itself, and a template for it, should be done in a new issue.
Comment #33
tic2000 CreditAttribution: tic2000 commentedI agree. The current block doesn't require a template file. But it could use a theme function.
Anyway, for that a new issue should be opened.
Comment #35
davidwhthomas CreditAttribution: davidwhthomas commentedThanks for this -dev commit, just what this module needed.
Tested and working fine. Even preserved the existing styling.
Looking forward to the recommended/stable release.
DT
Comment #36
ajlangsdon CreditAttribution: ajlangsdon commentedIn my site map, i am only posting the Primary & Secondary Links. There are about 100 primary and 20 secondary. Is it possible through theming or even using views to get 60 to display on one side and 60 on the other? It would display the first 60 links for primary in left column, and 61-100 in the right column. The 20 secondary would also be in second column. Any help on this would be excellent. Thanks.