bulletmenu generates wrong html ?

[ploc]

Hello,

I think that bulletmenu is generating wrong html code. This might be since an older release than the last beta 3, I can't tell.

The point is that when you're using submenus, you have to imbricate correctly sublists :


  menu 1
  menu 2
 
   
      menu 2.1
      menu 2.2
   
 
  menu 3


And it seems that the actual bulletmenu is generating the following html code :


  menu 1
  menu 2
 
    menu 2.1
    menu 2.2
 
  menu 3


You can see that sublists are not inside an tag...

Do you confirm it is a bug ?

[ploc]

In fact (I checked the xhtml standards), the right code should be :


  menu 1
  menu 2
   
      menu 2.1
      menu 2.2
   
 
  menu 3


You can notice that the sublist has to be included inside the parent

[nils73]

Henfe I modified bulletmenu to meet XHTML standards. I was also looking for a menu that does not show the active position in the hierarchy as a linked entry but with a class (let's say active). It was not really difficult to modify the code and clean it up a bit.

Regards,
Nils

[Ted]

nils73: Can you post your changes?  I'll put them into the main code base.  Saves me from having to redo it...

Thanks!

[iNSiPiD]

Bulletmenu already has a currentpage class attached to the for the active menu item. I just created a new class in my main CSS which reads:



currentpage {



    font-weight: bolder;

    text-decoration: none;

}



This gives the illusion that it's not  alink while also highlighting the active menu item.



Is it necessary to remove the link in the generated code? Maybe adding a paramter to {bulletlist} itself would be handier, as it provides the option.



I also thought (if you haven't already) that adding a class paramater to the multiple inline objects for pages would allow for the easy creation of custom types (events, job listings etc) via slight template modifications. Maybe it would even be possible to specify single/multi row for the fields and how big they should be, type of data accepted for verification etc. Kind of like a form builder. Just an idea.



I don't ask for much do I.

[nils73]

This one is just for wishy. I kept the dirty hacks for my own website (page_id) inside ... maybe you know a better solution.

Do not use this code unless you know what it does!!!

Code: Select all

<?php
#CMS - CMS Made Simple
#(c)2004 by Ted Kulp (wishy@users.sf.net)
#This project's homepage is: http://cmsmadesimple.sf.net
#
#This program is free software; you can redistribute it and/or modify
#it under the terms of the GNU General Public License as published by
#the Free Software Foundation; either version 2 of the License, or
#(at your option) any later version.
#
#This program is distributed in the hope that it will be useful,
#but WITHOUT ANY WARRANTY; without even the implied warranty of
#MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
#GNU General Public License for more details.
#You should have received a copy of the GNU General Public License
#along with this program; if not, write to the Free Software
#Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA

function smarty_cms_function_bulletmenu($params, &$smarty) {

	global $gCms;
	global $db;
	global $config;

	# getting menu parameters
	$showadmin = isset($params['showadmin']) ? $params['showadmin'] : 0 ;
	$collapse = isset($params['collapse']) ? $params['collapse'] : 0 ;

	$allcontent = ContentManager::GetAllContent(false);

	# defining variables
	$menu = "";
	$last_level = 0;
	$count = 0;
	$in_hr = 0;

	# array to hold hierarchy postitions of disabled pages
	$disabled = array();

	if (count($allcontent))
	{
		$basedepth = 0;
		// $menu .= "<div class=\"bulletmenu\">\n"; (I do not like DIVs ...)

		#Reset the base depth if necessary...
		if (isset($params['start_element']))
		{
			$basedepth = count(split('\.', (string)$params['start_element'])) - 1;
		}

		foreach ($allcontent as $onecontent)
		{
			#Handy little trick to figure out how deep in the tree we are
			#Remember, content comes to use in order of how it should be displayed in the tree already
			$depth = count(split('\.', $onecontent->Hierarchy()));

			#If hierarchy starts with the start_element (if it's set), then continue on
			if (isset($params['start_element']))
			{
				if ((strpos($onecontent->Hierarchy(), (string)$params['start_element']) === FALSE) || (strpos($onecontent->Hierarchy(), (string)$params['start_element']) != 0))
				{
					if (isset($params['show_root_siblings']) && $params['show_root_siblings'] == '1')
					{
						# Find direct parent of current item
						$curparent = substr($onecontent->Hierarchy(), 0, strrpos($onecontent->Hierarchy(), "."));
						if ($curparent != '')
						{
							$curparent = $curparent . ".";
						}

						# Find direct parent of start_element
						$otherparent = substr((string)$params['start_element'], 0, strpos((string)$params['start_element'], "."));
						if ($otherparent != '')
						{
							$otherparent = $otherparent . ".";
						}

						# Make sure the parents match
						if ($curparent != $otherparent)
						{
							continue;
						}
					}
					else
					{
						continue;
					}
				}
			}

			#Now check to make sure we're not too many levels deep if number_of_levels is set
			if (isset($params['number_of_levels']))
			{
				$number_of_levels = $params['number_of_levels'] - 1;

				#If this element's level is more than base_level + number_of_levels, then scratch it
				if ($basedepth + $number_of_levels < $depth)
				{
					continue;
				}
			}

			# Check for inactive items or items set not to show in the menu
			if (!$onecontent->Active() || !$onecontent->ShowInMenu())
			{
				# Stuff the hierarchy position into that array, so we can test for
				# children that shouldn't be showing.  Put the dot on the end
				# since it will only affect children anyway...  saves from a
				# .1 matching .11
				array_push($disabled, $onecontent->Hierarchy() . ".");
				continue;
			}

			$disableme = false;

			# Loop through disabled array to see if this is a child that
			# shouldn't be showing -- we check this by seeing if the current
			# hierarhcy postition starts with one of the disabled positions
			foreach ($disabled as $onepos)
			{
				# Why php doesn't have a starts_with function is beyond me...
				if (strstr($onecontent->Hierarchy(), $onepos) == $onecontent->Hierarchy())
				{
					$disableme = true;
					continue; # Break from THIS foreach
				}
			}

			if ($disableme)
			{
				continue; # Break from main foreach
			}

			# Set depth to be the relative position
			$depth = $depth - $basedepth;

			# Now try to remove items that shouldn't be shown, based on current location
			if ($collapse == 1)
			{
				if ($depth > 1) # All root level items should show 
				{
					$curpos = $gCms->variables['position'];
					$curdepth = count(split('\.', $curpos)) - $basedepth;
					$curparent = substr($gCms->variables['position'], 0, strrpos($gCms->variables['position'], "."));
					if ($curparent != '')
					{
						$curparent = $curparent . ".";
					}

					$skipme = true;

					# Are we the currently selected page?
					if ($onecontent->Hierarchy() == $curpos)
					{
						$skipme = false;
					}

					# First, are we a direct decendant of the current position?
					if (strstr($onecontent->Hierarchy(), $curpos) == $onecontent->Hierarchy() && $curdepth == ($depth - 1))
					{
						$skipme = false;
					}

					# Now for the nasty part...  loop through all parents and show them and direct siblings
					if ($skipme)
					{
						$blah = '';
						$count = 1;
						foreach (split('\.', $curpos) as $level)
						{
							$blah .= $level . '.';
							if (strstr($onecontent->Hierarchy(), $blah) == $onecontent->Hierarchy())
							{
								if ($depth == ($count + 1))
								{
									$skipme = false;
									continue;
								}
							}
							$count++;
						}
					}

					# Ok, so should we skip this thing already?
					if ($skipme)
					{
						continue;
					}
				}
			}

			if ($onecontent->Type() == 'sectionheader')
			{
				if ($in_hr == 1)
				{
					$menu .= "</ul>\n";
					$in_hr = 0;
				}

				$menu .= "<div class=\"sectionheader\">".$onecontent->MenuText()."</div>\n"; // must this really be a DIV? ;-)

				if ($count > 0 && $in_hr == 0)
				{
					$menu .= "<ul>\n";
					$in_hr = 1;
				}
			}
			else
			{
				if ($depth < $last_level)
				{
					for ($i = $depth; $i < $last_level; $i++)
					{
						$menu .= "\n</li>\n</ul>\n";
					}

					if ($depth > 0)
					{
						$menu .= "</li>\n";
					}
				}

				if ($depth > $last_level)
				{
					for ($i = $depth; $i > $last_level; $i--)
					{
						if ($depth == 1)
						{
						  $menu .= "<ul>\n";
            }
            else 
            {
              // adding a class to each <ul> for better CSS handling ...
              $menu .= "\n<ul class=\"subNav0".($depth-1)."\">\n";
            }
					}
				}

				if ($depth == $last_level)
				{
					$menu .= "</li>\n";
				}

				if ($onecontent->Type() == 'separator')
				{
					// inline styles are bad ... but since I do not use separators, I do not care ...
					// might be better to have <li class="hr"><hr /></li> here and assign in CSS
					// li.hr { list-style: none; }
          // .hr hr { visibility: hidden; }
          $menu .= "<li style=\"list-style-type: none;\"><hr class=\"separator\"/>";
				}
				else
				{
				  $menu .= "<li";
					if (isset($gCms->variables['page_id']) && $onecontent->Id() == $gCms->variables['page_id'])
					{
						// activated navigation-items should not be links ... 
            $menu .= " class=\"active0".$depth."\"";
            // even more sophisticated to make active menu items visible for screenreaders as <h4> elemtnes
						$menu .= "><h4>".$gCms->variables['position'].$onecontent->MenuText()."</h4>";
					}
					else 
					{
  				if ($onecontent->Hierarchy() == $curparent)
					  {
              // very useful for sub-navigation, to keep the parent activated
              $menu .= " class=\"activeCat\"";
            }
          // a dirty hack just for my website because some pages (search, etc.) do not show up in menu, but require same treatment
          elseif ((empty($curparent) && $gCms->variables['page_id'] == "2") || (empty($curparent) && $gCms->variables['page_id'] == "33"))
					  {
              $menu .= " class=\"activeCat\"";
            }
					$menu .= "><a href=\"".$onecontent->GetURL()."\">".$onecontent->Hierarchy();
          $menu .= $onecontent->MenuText()."</a>";
          }
				}

				$in_hr = 1;
				$last_level = $depth;
			}
			$count++;
		}

		for ($i = 0; $i < $last_level; $i++) $menu .= "</li>\n</ul>\n";

		if ($showadmin == 1)
		{
			$menu .= "<ul><li><a href='".$config['admin_dir']."/'>Admin</a></li></ul>\n";
		}
		// $menu .= "</div>\n"; ... my DIV-paranoia ends here ... ;-)
	}

	return $menu;

}

function smarty_cms_help_function_bulletmenu() {
	?>
	<h3>What does this do?</h3>
	<p>Prints a bullet menu.</p>
	<h3>How do I use it?</h3>
	<p>Just insert the tag into your template/page like: <code>{bulletmenu}</code></p>
	<h3>What parameters does it take?</h3>
	<p>
	<ul>
		<li><em>(optional)</em> <tt>showadmin</tt> - 1/0, whether you want to show or not the admin link.</li>
		<li><em>(optional)</em> <tt>collapse</tt> - 1/0, whether you want to collapse sub items that shouldn't be shown.  Defaults to 0.</li>
		<li><em>(optional)</em> <tt>start_element</tt> - the hierarchy of your element (ie : 1.2 or 3.5.1 for example). This parameter sets the root node of the menu and only shows it and it's children.</li>
		<li><em>(optional)</em> <tt>show_root_siblings</tt> - 1/0, if start_element (above) is given, then show direct siblings of the give start_element as well.</li>
		<li><em>(optional)</em> <tt>number_of_levels</tt> - an integer, the number of levels you want to show in your menu.</li>
	</ul>
	</p>

	<?php
}

function smarty_cms_about_function_bulletmenu() {
	?>
	<p>Author: Julien Lancien<calexico@cmsmadesimple.org></p>
	<p>Version: 1.0</p>
	<p>
	Change History:<br/>
	None
	</p>
	<?php
}

# vim:ts=4 sw=4 noet
?>

[ploc]

Henfe I modified bulletmenu to meet XHTML standards. I was also looking for a menu that does not show the active position in the hierarchy as a linked entry but with a class (let's say active). It was not really difficult to modify the code and clean it up a bit.

Regards,
Nils

Hello,

If we want to get closer to the xhtml standards and semantic xhtml, we should also modify the following point :

Section header text should be replaced by a simple Section header text

and

should be replaced by a simple

It is useless to create a class when a container (such as h1 or hr) already exists and can be set with css. It is also better to set every styles in css files and not in html files.

Do you agree ?

In fact I'm hardly working on a generic template and it appears that the generated code could be really improved and become lighter.

Ploc

[Ted]

I can get behind that...  I was doing things so that it looked right out of the box, but I can see a point in trying to do it "right".

[Ted]

Umm...  I just did an xhtml validation on bulletmenu and it says it's correct.

Code: Select all

<div class="bulletmenu">

<ul>
    <li><a href="index.php?page=Home_Page" class="currentpage">Home Page</a>
        <ul>
            <li><a href="index.php?page=frequently-asked-questions">FAQ</a></li>
            <li><a href="index.php?page=contact-us">Contact Us</a></li>
            <li style="list-style-type: none;"><hr class="separator" /></li>
            <li><a href="index.php?page=svn-cvs-">SVN (cvs)</a></li>
        </ul>
    </li>
</ul>

</div>

Am I losing my mind?

[iNSiPiD]

It is. You're not. I believe ploc confirmed that earlier.

[ploc]

Umm...  I just did an xhtml validation on bulletmenu and it says it's correct.

Am I losing my mind?

No you're not, I never said that the code was not valid. I was just pointing the thing that it is better to get closer to the semantic html : the structure in the html file, the stle in css file (but I think you already know that as it is the main advantage of cmsms).

So, I think that it is better to include all the style code in the css file. The thing is that if I want to change the style of the separator item in bulletmenu, I can't do that in the css file as it is already set in the

Same thing for the class="separator"...

Same thing again for the section header that does not need to be a special where a simple ... is enoug...

Do you agree ?

If I can try to sum up, as we already have a at the beginning, we don't need to use other divs and classes as we juste have to use usual html element whose appearance are set in the css file. I don't know if this makes my opinion clear, but you can see an exemple of what I mean in the following example page :

http://ploc.free.fr/public/newtemplate/main.html

[Ted]

No, I agree about the fact that the extra cruft should be taken out.  I'm just talking about the original post of it generating the wrong html...

Or maybe I'm missunderstanding post #2.  Was that a "nevermind"?

[ploc]

No, I agree about the fact that the extra cruft should be taken out.  I'm just talking about the original post of it generating the wrong html...

Or maybe I'm missunderstanding post #2.  Was that a "nevermind"?

Ok wishy, I did not realized that you were speaking about post #2.

In fact, if you take a look at my post #1, the code is not xhtml valid. The correct code is the one from the post #2.

Then nils73 has corrected the  bulletmenu function, this is maybe why the code is now xhtml valid.

As a conclusion, if it is valid, it's ok ! (except the points from my last post...)

Ploc

[Ted]

Well, this is the first time I actually got around to looking at the problem.  I actually haven't touched nils73's code yet...

So, that leads me back to: Am I losing my mind? 

[ploc]

Well, this is the first time I actually got around to looking at the problem.  I actually haven't touched nils73's code yet...

So, that leads me back to: Am I losing my mind? 

whishy, I don't think you're losing your mind. In fact, I think I've found an explanation...

In fact, I base the first post of this thread on the code generated by a demo version of cmsms (I can't remember if this is the demo cmsms website, or the one from opensourcecms.com) which is an old version of cmsms.

I think, the code had been corrected in en 0.9 or the 0.10 version...

Have you now found back your mind ?

Ploc

Edit : I've juste checked the demo version of cmsms website, the html code seems to be ok. I must have been smoke... (or the wrong html code came from opensourcecms.com...)

Anway, the last posts are still valid; but the firsts ones can be ignored I think...