Use orthogonal data for DataTables

[alistair3149]

This will make DataTables easier to expand and maintain. Now we can assign different attributes to the data, and this will resolve any sorting and filtering issues that we had.

Added

Instead of assigning HTML directly as data, we can assign an object with the following keys:

• display - The display value of the data (same as what we had)
• filter - Value used for filtering (e.g. SearchBuilder)
• sort - Value used for sorting

Removed

• any-number - Not needed anymore because sort key are always sortable
• datatables-column.type - Same reason as above

https://datatables.net/manual/data/orthogonal-data

Fix #831

[JeroenDeDauw]

Looks good. @thomas-topway-it can you also check this?

[malberts]

The code change looks fine, although I didn't do any manual tests.

Are there any possible backward compatibility issues here? Since this changes what the PHP outputs, I imagine pages with outdated caches will serve the old HTML with the new Javascript and then break. (Again, not manually confirmed, just checking.)

[alistair3149]

The code change looks fine, although I didn't do any manual tests.

Are there any possible backward compatibility issues here? Since this changes what the PHP outputs, I imagine pages with outdated caches will serve the old HTML with the new Javascript and then break. (Again, not manually confirmed, just checking.)

It would not be backward compatible and require a parser purge on the affected pages.

[thomas-topway-it]

for some reason I've missed this pull request until today. I've still to test it by the way it is great that you are moving to use orthogonal data (which as far as I understand are suitable to display structured data, like sub-objects in the context of SMW).

Regarding the sorting enhancement it seems an optimal solution, I will test with one of our demo (with the sorting relying on any-number to double check how it works)

@malberts you are correct, the old html may break the new javascript, however I don't see this as a "backward compatibility issue", just the requirement to update the pages

[JeroenDeDauw]

@thomas-topway-it so you will test this soonish?

[thomas-topway-it]

@JeroenDeDauw I cannot access the demo server currently, however it looks a cutting-edge solution, so if there is any issue I or @alistair3149 can fix it afterwards

[thomas-topway-it]

this can also be merged https://github.com/SemanticMediaWiki/SemanticResultFormats/compare/master...OpenSemanticLab:SemanticResultFormats:dev?expand=1

but I guess it should be rebased @simontaurus

[alistair3149]

this can also be merged https://github.com/SemanticMediaWiki/SemanticResultFormats/compare/master...OpenSemanticLab:SemanticResultFormats:dev?expand=1

but I guess it should be rebased @simontaurus

Is it the same as this PR? #861

[simontaurus]

Is it the same as this PR? #861

https://github.com/SemanticMediaWiki/SemanticResultFormats/compare/master...OpenSemanticLab:SemanticResultFormats:dev?expand=1
contains both
https://github.com/Knowledge-Wiki/SemanticResultFormats/tree/datatables-fix-template-parameter-ajax
and
https://github.com/simontaurus/SemanticResultFormats/tree/patch-1

[alistair3149]

@thomas-topway-it would you be able to review the PR?

[thomas-topway-it]

@alistair3149 I'm testing, a few minutes ... I have seen the new theme, is really beautiful but shouldn't be enabled via a parameter ?

[thomas-topway-it]

I have tested and think I understood how it works. Could we add

		if ( !count( $dataValues ) ) {
			return [
				'display' => $html,
				'filter' => '',
				'sort' => ''
			];
		}

or something similar before

$sortKey = $dataValues[0]->getDataItem()->getSortKey();

since $dataValues could be empty (it triggered an error on my test article)

Also, the idea is really clever, and of course the best solution so I totally agree to commit it, however how pseudo numeric titles are handled, like :

-- 1_Some title
-- 2_Some title
etc. ?

I think there was something similar in one of our demos ...

[alistair3149]

@alistair3149 I'm testing, a few minutes ... I have seen the new theme, is really beautiful but shouldn't be enabled via a parameter ?

We can either add a MW config key to disable that globally or/and add a datatable parameter to disable that . We did some modifications on both the styles and script to build the UI so we might have to refactor the changes together into the same place.

[alistair3149]

since $dataValues could be empty (it triggered an error on my test article)

It should be handled now. Would you be able to test it again?

however how pseudo numeric titles are handled, like :
-- 1_Some title
-- 2_Some title
etc. ?
I think there was something similar in one of our demos ...

I'm not quite grasping the concept. Would you mind elaborating further?

[codecov]

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Please upload report for BASE (master@33eaad7). Learn more about missing BASE report.
Report is 33 commits behind head on master.

Files with missing lines	Patch %	Lines
formats/datatables/SearchPanes.php	0.00%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master     #865   +/-   ##
=========================================
  Coverage          ?   45.51%           
  Complexity        ?     2320           
=========================================
  Files             ?       80           
  Lines             ?     9043           
  Branches          ?        0           
=========================================
  Hits              ?     4116           
  Misses            ?     4927           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[thomas-topway-it]

I have tested, thanks a lot