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

DM-5722: Add table client-side sorting #55

Merged
merged 1 commit into from
Apr 12, 2016
Merged

Conversation

loitly
Copy link
Contributor

@loitly loitly commented Apr 9, 2016

You can see this in TablePanel options and fits header dialog.

height='calc(100% - 42px)'
sortInfo={sortInfo}
//tableStore={tableStore}
/>
);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the sorting functionality simplified FitsHeaderViewer a lot.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just test FitsHeader, the sorting works. Nice job. I have a lot to learn, especially, the lodash, I never though of using it. Look at Loi's code, I found that get is very handy for searching something.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which situations do we sort on the client? Does it have implications for XYPlot or image, connected to a table (because they are sharing highlightedIdx)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to sorting: I noticed that some comments in Fits header show as "null". It might be better to show no value instead.

@loitly loitly merged commit c4a791e into dev Apr 12, 2016
@robyww robyww deleted the DM-5722_table_client_sort branch April 14, 2016 20:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants