Instead of "Sorry, You don't have access to this page" something like "Only Admins can change the menue config"? (because it is a action and you can have access to the page but not to the menu config)

Instead of "type" i would use "menu_name" (seems more selfexplaining for me).

But that are only two small things. Otherwise I like it :-)

A few off-the-cuff comments:
I understand it's still very much a "concept" but...
- using an ini maintenance web interface (which I'm making anyway) is just as "safe" as using an interface to edit a database table: so that is not really an argument for using a table
- both the current (config) solution and yours have the disadvantage of not supporting a title attribute for the actual links; an ini web interface could easily support this
- an ini web interface cannot do any "type-specific" validation (everything is just a string) while your table solution would enable this (though such validation is still missing)
- to support different menus as well as title attributes with SQL instead if ini you'd probably need two tables: one to store name and type of teh menu, another to store the data for each link
- when you use "type" that word suggests "class" but since "type" is a unique key and content is (therefore) also unique, "type" doesn't seem to be the correct name: that word suggests you can have multiple menus of the same "type"; I agree with Nils that "name" is probably better; still, in the generated HTML it should correspond to a class instead of an ID to enable to have the _same_ menu both at the top and at the bottom of the page (for instance)
- apart from the missing validation, your configuration interface has hard-coded types instead of picking these up from what is actually in the table; you could loop through them or have an action parameter to choose one and then present the interface to edit only that one menu; the action parameter itself would then need to be validated -OR- a non-existing menu name would lead to a form to actually create a new menu (which is missing now), with a note that no menu of that name was found; anyway there needs to be an interface to create a new menu, and delete an existing one, apart from merely editing what's in the table already

On balance I think this idea might indeed be better than having the menu definition in a config/ini file - but it needs work (also in separating logic from interface - mixing it all up makes it hard to understand and maintain). Nice job though! (Especially in convincing me a table solution is probably better than keeping it in an in file ;o))

> using an ini maintenance web interface (which I'm making anyway) is just as "safe" as using an interface to edit a database table

True, but the "ini" in question is the wakka system configuration, or you were thinking of a menu.config.inc?

> oth the current (config) solution and yours have the disadvantage of not supporting a title attribute for the actual links

The menu configuration interface just uses standard Wikka formatting. I think the right place to discuss about title attributes in links (which are a good idea, indeed) is the wikka formatter.

> types, classes, id's

I agree "type" is a bad word. I just wanted to wait for some reactions before deciding whether menus must have unique occurrences on the page, or if multiple occurrences are allowed (I think two distinct tables for menu categories and menu content could be avoided).
And yes, the menu name should be retrieved/stored from the table and not hardcoded like in my very first draft, I'll fix this ASAP.

btw, does any function use the GetConfigValue function instead of things like $this->config["table_prefix"]."?

@Nils
"does any function use the GetConfigValue"

Yes, I've seen this a lot (without doing global search, but I've seen it in multiple locations); I suspect it's a precursor of the more direct $this->config['some_name']. It would probably be a good idea to standardize this to the latter method (and then maybe get rid of the GetConfigValu() altogether so we don't keep bouncing back).

Should be part of a "code cleanup" action, I think.

@DarTar
"> using an ini maintenance web interface (which I'm making anyway) is just as "safe" as using an interface to edit a database table
True, but the "ini" in question is the wakka system configuration, or you were thinking of a menu.config.inc?"

I meant a general config ini file for Wikka instead of the current file with an array, and read/write/maintenance functions to handle such files. It would also enable one to easily put the configuration "above" the web document root and so make it more secure. PHP does have some functionality to read an ini file, but it's very poor (and I've found it uses this internally for the ini-get() function as well - I just saw it fall over in a bad way this morning after I changed a setting in my php.ini...). So I'm writing my own class (set of classes, really) to handle ini files. Will handle and maintain even comments. Integration with Wikka will be easy, I promise!
(And if Wikka doesn't adopt it, I can still use it for a lot of other things ;-))

Still, I now like your idea of using a table rather than a config file (in whatever form) to store menu data, with a dedicated UI for it.

@JW:
Ok, just wanted to know which method should be preferred (I remember vaguely reading some things about getting variables through functions and not calling it directly, but that was some years ago and I don't remember any details).

@Nils,
"I remember vaguely reading some things about getting variables through functions and not calling it directly"

Probably some advice about OO? Wikka is about "half" OO, I'd say. :) The idea which you probably read about is that an object encapsulates data and its methods and one should only be able to "get at" (read, write) an object's data via the methods the object exposes.

That said, all of the actions code is not a set of functions external to a (the) Wakka object, but bits of code that are included dynamically (in fact, not actual functions at all); any method (and thus also "part of" a method) that's internal to an object has direct access to all of the object's data.

That said, I'd have to check where exactly GetConfigValue() is used - maybe we do still need it: it depends where it's used (internally or externally from the object).

Good you're brining that up!

...

OK, at least one place where it's used from an "external" function - so we need to keep the function; but also places where it's used but not needed, actions, handlers, even in functions in the Wakka class file itself.

Yes, that was what I had in my mind (but you can explain better :)

Could you please give some more detail regarding changing the header.php file for the wikka menus.
What code has to be removed exactly?
Sorry, but I'm not a programmer.

James, you should use the new version of the code that I've just posted.
The block that has to be replace in header.php is the following (hope it's the original one):

<?php
if ($this->GetUser()) {
echo $this->config["logged_in_navigation_links"] ? $this->Format($this->config["logged_in_navigation_links"])." :: " : "";
echo "You are ".$this->Format($this->GetUserName());
} else {
echo $this->config["navigation_links"] ? $this->Format($this->config["navigation_links"]) : "";
}
?>

Thanks DarTar I'll try the new version.
The old one was giving me error messages.
I don't know if it was me (most likely) or an error in the code or a config problem.

There were actually two typos: apologies. Check also the new version of the header replacement code. This should work - good luck ;-)

DarTar,
I tried the modified header but I'm still getting an error.
Fatal error: Call to undefined function: printmenu() in /home/www/xxx/xxx/actions/header.php on line 43

Can you offer any help.

Just make sure that you've copied and pasted *all* the functions listed in step 2. of the installation into your wikka.php file.

I think James did not copied the menu functions at the right place: the first block of functions should be copied after the Wakka class declaration; actually after:
------------------------------------
class Wakka
{
var $dblink;
var $page;
var $tag;
var $queryLog = array();
var $interWiki = array();
var $VERSION;
------------------------------------

I have now copied and pasted *all* the functions and the menu works fine.
Thanks DarTar and Christian.

@Christian & James:
It's not a good idea to have methods *before* the constructor in a class definition. While the menu functions surely must be *within* the class definition they should probably be withina section of their own, much as we already have otehr sections marked with // comments. I'd recomment creating a //MENUS section to place the functions in.

@DarTar:
- The forms look somewhat unwieldy - no need to create such *big* buttons with "menu' in them when it's already clear the form is *for* a menu. Just Update, Delete and Rename should do fine.
- CSS: why hide the exttail at all? And if you do want to hide it, why not use display: none; instead of visibility: hidden; - so it won't take up any space either?

OK, points taken, I'll update them ASAP. But what about the general vision? (including the fact of modularizing the current menu chunks).

I certainly like the approach! Much more flexible and maintainable.

I'd already been wondering how to integrate the current "You are [Name]" and using the "mini actions" modules is a nice solution!

hmm .. I was wondering why not just dump all those miniactions DarTar proposes to one proper action ?

It could be like {{footer menus="edit,history,revisions,owner,referrers,search"}}

it could be equally practical and customisable i think.

for header menu there could be a similar proposal ...
{{header menus_logged="HomePage, Categories, PageIndex, RecentChanges, RecentlyCommented, usersettings(Change Settings/Logout), who" menus_notlogged="HomePage, CategoryCategory(Categories), RecentChanges, PageIndex, UserSettings(Login / Register)}}

Note:
{{menu_name menu_state="target_page_name(display_text), another_page_name(display text for 'another page name'), ... and so on. .. }}

I hope it is clear ..

George, if you like this GUI-based approach to custom menus, having menulets (http//wikka.jsnx.com/WikkaMenulets) handled by the same action might be a good idea: {{menu show="user"}}, {{menu show="edit"}} , although they become less intuitive to use, and I still prefer one name for one action.

OTOH, I don't like that much the idea of coding the whole menu content as a single comma-separated action parameter. Using menu tables gives you much more flexibility than just using action parameters. For instance, how would you embed an existing action in an action parameter? And isn't a GUI for menus more user-friendly than your header menu action?

surely positive that a gui is the way forward,.. it can interface anything we want really ... what I propose isnt blocking the use of a page-gui :)
all that the gui will be doing is interfacing the attributes and the values of the action...

"how would you embed an existing action in an action parameter?"

Maybe this could do it ..
{{menu_name menu_state="PageName(Display text for PageName), ![action_here]! , ...}}

I just thought it would be a bit more practical,.. but if it is not doing it then its fine with me ;)

The disadvantage I see in your approach is that it uses a lot of non-standard tags:

- PageName(Text) for links
- ![action]! for actions

which require dedicated parsers and further learning for the user.

My approach OTOH only uses standard Wikka Syntax. No need of further parsers and the syntax is already known to the user:

[[PageName Text]]
{{action}}

Moreover, I don't understand why you are suggesting an "action". Where would you use such an action? In every page? In the page header/footer? Your proposal looks more like an extension of the current menu entries in wikka.config.php: I've already tried to convince JavaWoman of the advantages of having menus handled by tables instead of config files (take a look at the comments above).

it was the header.php and the footer.php files I was aiming at... arent those included automatically for every page ? (asking ... I dont really know),..
about the syntax yes it is true it can be hard to make,.. although a gui could overcome this..
I am on your side about quiting the configuration file.
( I think that the configuration file should only be needed to setup the connection to mysql variables,... the remaining conf vars should IMHO be in a mysql table (easier to interface, safer)

"I've already tried to convince JavaWoman of the advantages of having menus handled by tables instead of config files"
Well, the discussion was really about using a GUI to define menus: your action does provide that but a GUI for a config file could do it just as well. I like what you're doing (no opposition). But, I'm not convinced a table is _necessary_. But I did say already "On balance I think this idea might indeed be better than having the menu definition in a config/ini file"

I still don't see support for providing a title attribute though. That's really needed, I think.

George,
Actually, currently header.php and footer.php are themselves actions (they're in the actions directory!) - not "source" files that could be edited to themselves include actions.

That said, I'm not conviced they should _be_ (or be treated as) actions; looking at the code they could just as easily be "includable" files. Which mean they don't really belong in the actions directory either. An extra argument is that while you could include {{header}} or {{footer}} in a page, doing the former would actually create a totally invalid "HTML" document, since it not only generates the visible page header with the menu, but also the DOCTYPE declaration and the head section of the document.

Real actions should be written in such a way as to allow inclusion in a page *without* causing invalid (X)HTML.

sorry if I wasnt clear before ..

now there are header_action and footer_action in the config ...
they provide to assign the strings that define the actions to include anyway.

if we take for granted this in the config:
'footer_action' = '{{footer menus="edit,history,revisions,owner,referrers,search"}}
I was thinking that alterning the Header() and Footer() in the core to smth like

function Footer() { echo $this->Format($this->config['footer-action']) }
instead of $this->Action($this->config['footer-action'])

it should call the footer.php as it does now but should give a bit more flexibility..

on the issue of {{footer}} inside a wikipage :
I suggest to do this simple patch to keep people from including the footer.php or header.php files in their wikipages ...
/* in formatters/wakka.php */

// Actions
else if (preg_match("/^\{\{(.*?)\}\}$/s", $thing, $matches))
{
if ($matches[1]) {
return (substr($matches[1],0,6)==('header' || 'footer')) ? '{{}}' : $wakka->Action($matches[1]) ;
}
}

instead of the existing code that doesnt have any prevention 'if' ...
// Actions
else if (preg_match("/^\{\{(.*?)\}\}$/s", $thing, $matches))
{
if ($matches[1])
return $wakka->Action($matches[1]);
else
return "{{}}";
}

"I still don't see support for providing a title attribute though. That's really needed, I think."

JW, as I said before, I completely agree bu I think this is not an issue to be discussed here: WikkaMenus only apply Wikka formatting rules to menu entries, so if there is a title issue it has to be addressed here: http://wikka.jsnx.com/WantedFormatters.

As for header.php and footer.php:
<what JavaWoman said>

Not a "wanted formatter" though but a *change* to the current formatter handling for (forced) links. Currently only {{image}} provides a way to add a title attribute.

And your "menulets" are actions and thus could provide title attribute support in the same way the {{image}} action does.

BTW, main_menu could optionally also contain {{who}} - does your menulet for that support displaying remote address for a not-logged-in user?

Thanks for developing WikkaMenus! I made an extension implementing a switch for changing the menu item language (and the pages the menu links refer to). It's on MultiLanguageMenus.

Is there any specific reason to use a "varchar(255)" for wikka_menus? My menu is bigger than 255 bytes and gets truncated.

I changed to type to "mediumtext" and increased the size. It works fine but wanted to check with you if you see any problems.

Hi Dac, tere shouldn't be any problems. I chose varchar(255) for no specific reason other that menu items are usually shorter than 255 characters and I cannot imagine cases with such long items, could you give us an example?

Hi DarTar,

something like that

[[Home Home]] ::
[[PageIndex Sitemap]] ::
[[CategoryCategory Categories]] ::
[[RecentChanges Changes]] ::
[[RecentlyCommented Comments]] ::
[[UserSettings Profile]] ::
[[MyPages My Pages]] ::
[[http://wiki.blahblah.de/UserSettings?action=logout Logout]]

If I want to include other "direct links" (e.g. Logout) which require the full url, I need more characters than (just) 255.

right, I thought the 255 string max. length was per item. It definitely looks a bit too short for a whole menu.

btw, consider that you don't need to add separators (e.g. ":") anymore if you use this extension. Separators are taken care of in the stylesheet, see http://css.openformats.org/css/wikka.css

Thanks for the hint. I am no CSS expert, therefore I added those separators manually. Will try your fix :)

----

Cool! It works :)

:-( The separators won't show up in IE 5/6 ... grrrr ... I think I will add them manually in the via the menu action.

The idea of moving the separators to the stylesheet is because they are part of the presentation and don't have any semantic role. Instead of adding them manually to the source, I'd get myself a real browser.

You don't have to convince me - I am using FireFox since version 0.0 :) . But if I take a look at the user distribution of my other websites still about 60-65% use the Internet-Explorer and therefore the page would look "weird" for about 60-65% of the users.

I agree with you that using CSS is the best solution but IE has unfortunately some major bugs with the CSS interpretation.