Item2066: latest TablePlugin changes breaks theming of foswikiTables

Formerly the css cascaded as followed:

1. settings from TABLEPLUGIN_TABLEATTRIBUTES
2. settings from the skin's css
3. settings as defined by a %TABLE setting in front of a table.

This is broken now. As TablePlugin preferences are treated like user input now, they both end up overruling a skin's css. The default parameter for TablePlugin only look well on PatternSkin, but not on any other skin that comes with its own foswikiTable attributes.

-- MichaelDaum - 14 Sep 2009

Worked around it in latest NatSkin. Other skin authors might need to take care of this similarly by explicitly adding their css _after TABLEPLUGIN_default using ADDTOHEAD.

-- MichaelDaum - 14 Sep 20

I could try to put the table skin css first with addToHEAD. Then the skin's css would come after and overwrite the styles.

-- ArthurClemens - 14 Sep 2009

Hm. There's no way to express "comes first", there's only "comes after X".

-- MichaelDaum - 14 Sep 2009

Unless with a hack in Foswiki.pm:

    $this->{_HTMLHEADERS} = {};
    $this->addToHEAD('FIRST', '<!---->');

-- ArthurClemens - 14 Sep 2009

I already worked around it in NatSkin. Sorry for my first harsh reaction on your changes on TablePlugin. I've been able to work around it by explicitly requiring TablePlugin_default while adding NatSkin's css to the head. This works fine now. Other things that needed adjustments is your changed way to classifiy markup, e.g. sorted tables etc.

Hacking the core to add a "comes before X" in addition to "comes after X" and even a "comes first" and "comes last" might be quite valuable.

But that's no option for me right now because I need a solution that is backwards compatible to:

1. Foswiki 1.0.6, old TablePlugin
2. Foswiki 1.0.7, new TablePlugin and then comes
3. Foswiki 1.1, using yet a different TablePlugin using some new "comes first" feature in the core ... (where 1.1 is actually feature-freezed as far as I remember?)

Frankly, the concept behind TablePlugin, i.e. sorting tables on the server-side, is pretty outdated. We should work on client-side sorting features instead of refactoring it yet again without a definite need.

-- MichaelDaum - 15 Sep 2009

True about sorting. There probably is something in jquery for that, isn't it?

Otherwise, to keep sorting serverside and client side on par, it might be possible to create sorted lists serverside for every column, and write these as lists to the head.

-- ArthurClemens - 15 Sep 2009

Michael (and others). The refactoring of the TablePlugin with respect to CSS will not be released in 1.0.7. It is only the bug fixes that are merged over. Not the CSS changes. And I will not merge them over and I will revert if others do.

That change needs to "settle" in 1.1 context so you and others have the time to "absorb" the change.

The only enhancement I have done is Wysiwyg because the new TMCE resolves many old editor bugs. I regard it as a bug fix as the feature set to the end user is the same.

Kenneth

-- KennethLavrsen - 15 Sep 2009

@Arthur: here's one http://tablesorter.com which seems straight forward to add to JQueryPlugin.

There are some more out there that need to be evaluated like these grid plugins: http://wiki.jqueryui.com/GridTable (work in progress), http://www.flexigrid.info/ or http://www.trirand.com/blog/.

These plugins cover functionality client-side in a quite elegant way compared to our two separate server-side plugins TablePlugin and EditTablePlugin.

-- MichaelDaum - 15 Sep 2009

Reopening this bug item again as a consequence of distro:0d0d946f4765

We do need to agree on a identifier like TABLEPLUGIN_default for the default settings TablePlugin adds to the head.

Otherwise skins have no chance to override them.

-- MichaelDaum - 16 Sep 2009

The reasoning behind the change is to combine all styles, to prevent IE from crashing the CSS rendering (see Item2050).

So what you need is:

1. Styles in TABLEPLUGIN_default
2. All other styles in TABLEPLUGIN_specific

There is a bug that tables from included topics do not get their style added to head.

-- ArthurClemens - 16 Sep 2009

Right, and skin css should go in between ... hopefully. In fact the current "requires" constraint does not grant that

%ADDTOHEAD{text="skin css" requires="TABLEPLUGIN_default"}%

will be put there. This happens so just by accident.

It could very well be placed after TABLEPLUGIN_specific. However that's not what we want.

NatSkin for instance adds its css using a NATSKIN identifier. To grant that TABLEPLUGIN_specific comes after it you'd have to add a "requires" constraint to the TABLEPLUGIN_specific block. The solution would be to agree on a SKINCSS identifier and use it in TablePlugin and NatSkin.

-- MichaelDaum - 16 Sep 2009

Fixed in Item2050. The 2 CSS sections are TABLEPLUGIN_default and TABLEPLUGIN_specific (and should not change name).

-- ArthurClemens - 17 Sep 2009

Reopening this bug item as the reported issue is not solved reliably as discussed above.

Why: Skins need to put their css between TABLEPLUGIN_default and TABLEPLUGIN_specific. Alas, a simple

ADDTOHEAD{"SKINCSS" text="..." requires="TABLEPLUGIN_default"}

only grants that the SKINCSS is put somewhere after TABLEPLUGIN_default. The ADDTOHEAD could very well decide to put it even after TABLEPLUGIN_specific.

Using a simple "requires" constraint, we need the following patch to the TablePlugin:

--- lib/Foswiki/Plugins/TablePlugin/Core.pm     (revision 4973)
+++ lib/Foswiki/Plugins/TablePlugin/Core.pm     (working copy)
@@ -1179,7 +1179,7 @@
 </style>
 EOS
     $header =~ s/(.*?)\s*$/$1/;    # remove last newline
-    Foswiki::Func::addToHEAD( $inId, $header, $HEAD_ID_DEFAULT_STYLE );
+    Foswiki::Func::addToHEAD( $inId, $header, "SKINCSS, $HEAD_ID_DEFAULT_STYLE" );
 }

Only then SKINCSS is granted to be placed (a) after TABLEPLUGIN_default and (b) before TABLEPLUGIN_specific.

The other option to add an additional "before" constraint is better in terms of less agreements between plugins and skins needed but would be a lot more flexible, e.g. writing

ADDTOHEAD{"SKINCSS" 
  text="..." 
  after="TABLEPLUGIN_default"
  before="TABLEPLUGIN_specific"
}

(requires renamed to after for consistency)

-- MichaelDaum - 18 Sep 2009

Added proposal in Item2095.

-- ArthurClemens - 18 Sep 2009