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

Allow table sorting #245

Closed
wants to merge 5 commits into
base: trunk
from

Conversation

Projects
None yet
2 participants
@loganmzz

loganmzz commented Jan 1, 2017

It is a PR about Bugzilla ID 52962

PR is cut into many commits for each required step for full implementation. Final commit enables sorting for aggregate report, view results in table and summary report.

Any table relying on ObjectTableModel must be supported.

@pmouawad

This comment has been minimized.

Show comment
Hide comment
@pmouawad

pmouawad Jan 3, 2017

Contributor

Hello,
Would it be possible to provide a version that has no conflict with TableVisualizer.java ?

There was some work on Sonar quality report, but I'll pause it on my side until this PR is merged if rest of the team is ok with merging.

Thanks

Contributor

pmouawad commented Jan 3, 2017

Hello,
Would it be possible to provide a version that has no conflict with TableVisualizer.java ?

There was some work on Sonar quality report, but I'll pause it on my side until this PR is merged if rest of the team is ok with merging.

Thanks

loganmzz added some commits Jan 3, 2017

Bug 52962 - Relies on default table header renderer
It makes possible to use fully featured table header such as sort icon
support.
Bug 52962 - Fixes fired event row indexes
Last row index is INCLUSIVE ! It causes IndexOutOfBounds when using
table sorter.
Bug 52962 - Adds table sorting
Available on components: aggregate report, view results in table and
summary report
@loganmzz

This comment has been minimized.

Show comment
Hide comment
@loganmzz

loganmzz Jan 3, 2017

I have commented TableModelEventBacker, and rebase it on trunk: d29ac21 (Sonar : fix squid:UselessParenthesesCheck Remove those useless parentheses.)

You must force fetch. Or delete remote branch references and refetch it.

loganmzz commented Jan 3, 2017

I have commented TableModelEventBacker, and rebase it on trunk: d29ac21 (Sonar : fix squid:UselessParenthesesCheck Remove those useless parentheses.)

You must force fetch. Or delete remote branch references and refetch it.

asfgit pushed a commit that referenced this pull request Jan 14, 2017

Bug 52962 - Allow sorting by columns for View Results in Table, Summa…
…ry Report, Aggregate Report and Aggregate Graph

Based on a contribution by Logan Mauzaize (logan.mauzaize at gmail.com) and Maxime Chassagneux
This closes github pr #245
Bugzilla Id: 52962

git-svn-id: https://svn.apache.org/repos/asf/jmeter/trunk@1778767 13f79535-47bb-0310-9956-ffa450edef68
@pmouawad

This comment has been minimized.

Show comment
Hide comment
@pmouawad

pmouawad Jan 14, 2017

Contributor

Hello,
I have commited part of the PR and made some changes:

  • I didn't want to use everywhere HeaderAsPropertyRenderer.install as it removes formatting without introducing sorting
  • It would have also broken backward compatibility on the class potentially breaking backward compatibility of plugins
  • So I have replaced it by a HeaderAsPropertyRendererWrapper class
  • I have added sorting to Aggregate Graph
  • In ObjectTableModel, I have also replaced the modified method by a new one getObjectListAsList to avoid breaking backward compatibility

Thanks for your PR, please test nightly build to see if it works fine for you.
Thanks

Contributor

pmouawad commented Jan 14, 2017

Hello,
I have commited part of the PR and made some changes:

  • I didn't want to use everywhere HeaderAsPropertyRenderer.install as it removes formatting without introducing sorting
  • It would have also broken backward compatibility on the class potentially breaking backward compatibility of plugins
  • So I have replaced it by a HeaderAsPropertyRendererWrapper class
  • I have added sorting to Aggregate Graph
  • In ObjectTableModel, I have also replaced the modified method by a new one getObjectListAsList to avoid breaking backward compatibility

Thanks for your PR, please test nightly build to see if it works fine for you.
Thanks

@asfgit asfgit closed this in 486c99b Jan 14, 2017

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