I don't know if there is an existing issue, so creating this on to record my current brain-storm.

An important goal is to not bloat the router table. So, I think the way to approach this problem for D6 is to use a set of wildcard paths plus path aliases

The paths could look something like: 'node/%/panels-tab/1', 'node/%/panels-tab/2', etc

Note: no loader specified for arg(1) on purpose to improve performance.

You would need a table, something like:

  $schema['og_panels_tabs'] = array(
    'description' => 'Tracks user-created tabs.',
    'fields' => array(
      'nid' => array(
        'description' => 'The primary identifier for a node.',
        'type' => 'serial',
        'unsigned' => TRUE,
        'not null' => TRUE),
      'tabnum' => array(
        'description' => 'The current {node_revisions}.vid version identifier.',
        'type' => 'int',
        'unsigned' => TRUE,
        'not null' => TRUE,
        'default' => 0),
      'title' => array(
        'description' => 'The title of this tab, always treated as non-markup plain text.',
        'type' => 'varchar',
        'length' => 255,
        'not null' => TRUE,
        'default' => ''),
      'tab_alias' => array(
        'description' => 'The part of the path alias after node/[nid].',
        'type' => 'varchar',
        'length' => 255,
        'not null' => TRUE,
        'default' => ''),
      'settings' => array(
        'description' => 'The panel settings.',
        'type' => 'text',),
      ),
    'indexes' => array(
      'tabnum'        => array('tabnum'),
      ),
    'primary key' => array('nid', 'tabnum'),
    );

You might seed this table w/ some rows o hook_install with nid == 0, and maybe tabnum 1,2, and 3 (more on this later).

og_panels_menu would then create router entries like:

"SELECT DISTINCT(tabnum) FROM {og_panels_tabs}"

foreach ($result as $tab) {
  $item["node/%/panels-tab/$tab->tabnum"] = array (
    ...
     'access callback' => 'og_panels_tab_access',
     'access arguments' => array(1,3),
  )
}

When the user wants to create a tab, for a node, you insert a row into this table with tabnum +1 more than the greatest existing one for that node. If you exceed the greatest tabnum already in the table, you have to trigger a menu rebuild (hence why you might seed the table)

You could have a helper function that loads all the info from this table for the current node into a static variable when it's 1st called.

Then suppressing non-existent tabs for the current node become easy - using the arg(1) and arg(3) just see if that table entry exists.

You could use a title callback to set the title of the tab/page, etc.

The weighting of the tabs would behind the scenes work as swapping tabnum/alias between items. Similarly if the user removes a tab, you'd probably want to compact the data so that there is no gap in tabnum.

Comments

moshe weitzman’s picture

moshe weitzman commented

At first glance this looks like a great approach that works right now. Very clever.

I'm not sure we should expect module developers to perform these gymnastics going forward.

joshk’s picture

joshk commented

I'm not sure if I'm understand this, or perhaps it's no longer a big deal.

The current fix #307980: Port OG Panels to D6 shouldn't bloat the router table. We're using menu_alter() to snap up the node/% callback, and then playing nicely with delegator stuff so you can ahve og_panels, panel_nodes and delegator overrides of a third node type (e.g. node/% overridden for stories) all playing in sweet harmony.

Now, I could see a case for this to all get rolled directly into delegator, with og_panels being a second override option, which would let you have some group node types run as panels and others not. This would be extra-awesome.

But none of that speaks to Peter's initial issue about router table bloat, which I don't see as being problematic with our current solution, or the possibility of a future og_panels delegator, with all its awesomeness. Am I wrong?

joshk’s picture

joshk commented

Just to be clear, when I say:

I could see a case for this to all get rolled directly into delegator

I don't mean adding code to delegator. I mean implementing the og_panels functionality as a delegator plugin. :)

joshk’s picture

joshk commented

Moshe reminded me what was up here: _og_panels_nodes_menu() is bad.

Will work on this shortly.

joshk’s picture

joshk commented

Word. Ok, so this means we only have N entries in menu_router for the most tabs ever on a given node. That's very clever!

The additional work will be to rock the url_aliasing, but that shouldn't be too hard. Probably i'll make "path" optional on the main form, and then only even add the element if path module is on. In theory, if you don't care about your urls, you can run this all on numeric ids.

merlinofchaos’s picture

merlinofchaos commented

It occurs to me that you don't need *any* menu entries.

node/%/foo will still get handled by node/%

task handlers can get the full set of arguments

It can peer at that argument to see *which* node to serve up based upon what it sees as 'foo'.

This can be *entirely* done with delegator, no menu router stuff at all, and no clunky aliasing.

merlinofchaos’s picture

merlinofchaos commented

actually node_view.inc may not pass the argument properly. That can be fixed

merlinofchaos’s picture

merlinofchaos commented

Oh and that won't display the tab. =(

I keep forgetting about that part. I realy wish we could dynamicly add tabs.

joshk’s picture

joshk commented

Hrmz... taking this on now, I'm thinking I might not even need this extra table. The gymnastics here are all for allowing tab titles, but I'm pretty sure I can drive this all on a numeric basis to start, and then expose pathauto tokens to make the urls nice and semantic. This seems a lot cleaner to me. Investigating.

merlinofchaos’s picture

merlinofchaos commented

CTools now has a method to do arbitrary tabs. It requires some level of cooperation from the theme, but that's not a really big deal I don't think.

joshk’s picture

joshk commented

Made good progress, but stuck on how to override the default node/view tab in cases where a "use as homepage" bit is set though...

http://drupal.org/node/307980#comment-1699552

merlinofchaos’s picture

merlinofchaos commented
Status: Active » Closed (won't fix)

Panels 2 is no longer available and is unsupported. Marking all Panels 2 issues won't fix.