Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SRF][Filtered] New view 'table' added #122

Merged
merged 9 commits into from
Sep 20, 2015
Merged

[SRF][Filtered] New view 'table' added #122

merged 9 commits into from
Sep 20, 2015

Conversation

gesinn-it-gea
Copy link
Member

At gesinn.it, we've been working internally with a "table" view enhancement for the filtered view since a while. Our customers love it. We've polished it a bit and merged it with a recent version of SRF.

Format is tested on MW 1.25.2, SMW 2.2.2. No issues.

We'd like to contribute this back to SRF so that others can use it...

@mitchelln
Copy link

That's really fantastic! Thanks for the effort.

@s7eph4n
Copy link
Contributor

s7eph4n commented Sep 8, 2015

Nice!
If somebody could give this patch a spin, that'd be great. Otherwise I'll do it next week and merge it then.

@kghbln
Copy link
Member

kghbln commented Sep 8, 2015

I am very excited about this. Very cool stuff! @gesinn-it It will be great if you could document this after the merge an smw.o. @JeroenDeDauw I guess this could also trigger a soonish SRF 2.3 release after merge?

@gesinn-it-gea
Copy link
Member Author

@kghbln : sure, I'll modify smw.o as soon as things are merged.

undone clean up default for 'class' parameter. Should have 'wikitable sortable'...
@kghbln
Copy link
Member

kghbln commented Sep 9, 2015

👍 And I will poke @JeroenDeDauw when time comes to update smw.o so the example will work allrighty. Very excited, I am.

@JeroenDeDauw
Copy link
Member

@kghbln sure, we can do a 2.3 rel

@mitchelln
Copy link

Any chance we could put the quick and/or default selection filter option in before a new release Stephan?

No worries if not.

Sent by Outlook for Android

On Wed, Sep 9, 2015 at 11:27 AM -0700, "Jeroen De Dauw" notifications@github.com wrote:
@kghbln sure, we can do a 2.3 rel


Reply to this email directly or view it on GitHub:
#122 (comment)

@JeroenDeDauw
Copy link
Member

I have submitted some small refactoring of this new code at https://github.com/gesinn-it/SemanticResultFormats/pull/1

(If it gets merged there, it will show up in this PR)

JeroenDeDauw and others added 2 commits September 11, 2015 05:15
This splits methods, removes not needed code and improves type inferrence.
Small refactoring of the new table view PHP code
@JeroenDeDauw
Copy link
Member

+1 from my side. Would be more happy if the code was tested of course. Ping @mwjames

@mwjames
Copy link
Contributor

mwjames commented Sep 15, 2015

Would be more happy if the code was tested of course. Ping @mwjames

Since I haven't tested nor did I look at the code, I have to excuse myself from the evaluation (or recommendation).

@s7eph4n Please merge at your discretion.

@gesinn-it-gea
Copy link
Member Author

@s7eph4n can you please have a look at this PR and merge?

@s7eph4n
Copy link
Contributor

s7eph4n commented Sep 20, 2015

Does not work for me. :(
snapshot1
snapshot2

@s7eph4n
Copy link
Contributor

s7eph4n commented Sep 20, 2015

@gesinn-it Could you try if #127 works for you?

It is a table view that I built for SRF 1.9 some time ago that seems to also work with SRF 2.2.
There were some issues when I tried this with 2.0 or 2.1, which is why I did not merge it to master back then, but it seems to work now. No idea what changed since then (or if I was chasing phantoms).

@gesinn-it-gea
Copy link
Member Author

@s7eph4n I can reproduce the issue that the table view is not working if multiple views are used. I'll have a look at it.

@gesinn-it-gea
Copy link
Member Author

SRF_FV_Table.php

... adding $resultArray->reset() fixed it for me:

protected function getCellForPropVals( SMWResultArray $resultArray, $outputmode, $columnClass ) {

                $resultArray->reset();

                $dataValues = array();

@s7eph4n Can you directly add this line and see if it is working for you as well?

Set the internal pointer of the array of SMWDataItem objects to its first element. This fixes an empty table issue in case the table view is added not as first view in a multiple views setting.
s7eph4n added a commit that referenced this pull request Sep 20, 2015
[SRF][Filtered] New view 'table' added
@s7eph4n s7eph4n merged commit 0000c70 into SemanticMediaWiki:master Sep 20, 2015
@s7eph4n
Copy link
Contributor

s7eph4n commented Sep 20, 2015

Looks good to me.
Now, who will add this view to http://semantic-mediawiki.org/wiki/Help:Filtered_format? ;)

@JeroenDeDauw
Copy link
Member

Can you update the release notes?

@gesinn-it-gea
Copy link
Member Author

@JeroenDeDauw created PR with updated rel notes

@kghbln
Copy link
Member

kghbln commented Sep 21, 2015

And it (#128) was merged. :)

@kghbln
Copy link
Member

kghbln commented Sep 21, 2015

@JeroenDeDauw Ready for SRF 2.3?

@gesinn-it-gea
Copy link
Member Author

@kghbln , @JeroenDeDauw we should have PR #126 merged before the next release...

@JeroenDeDauw
Copy link
Member

Tracking in #129

@mwjames
Copy link
Contributor

mwjames commented Sep 25, 2015

@gesinn-it Besides the docu on smw.org, it would be marvellous to have a real sound example on smw.org.

@kghbln
Copy link
Member

kghbln commented Sep 25, 2015

This will be nice indeed. I pinged @JeroenDeDauw to update smw.o Should not take to long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants