Sort variables in variable explorer by column #5876
Conversation
| this.setState({ | ||
| containerHeight: nextProps.containerHeight, | ||
| gridHeight: nextProps.gridHeight, | ||
| variables: nextProps.variables |
There was a problem hiding this comment.
If this.props.variables changes while a sort is active on the variable explorer's contents, would this cause the variable order to reset to an unordered state? I believe the user would expect their existing sort order to be preserved as variables change.
There was a problem hiding this comment.
good point, I'll take a look into it
| @@ -498,4 +516,22 @@ export class VariableExplorer extends React.Component<IVariableExplorerProps, IV | |||
| this.props.showDataExplorer(row.buttons.variable, row.buttons.numberOfColumns); | |||
| } | |||
| }; | |||
|
|
|||
| private sortRows(sortColumn: string, sortDirection: 'ASC' | 'DESC' | 'NONE') { | |||
There was a problem hiding this comment.
You should probably save the sortColumn/direction so that on refresh you can resort. Although that would mean sorting when set the state.
There was a problem hiding this comment.
Hmm, maybe not. Sorting would require fetching all rows so if you resorted on variable refresh that would likely make all variable refreshes fetch every row.
There was a problem hiding this comment.
Hmm, thinking about this more, it feels like the variables need to be sorted when requested from the kernel. This way you can still keep the paging algorithm
There was a problem hiding this comment.
And then you could save the sort column/direction, but instead of sorting in the UI, you'd make a new variable request to the server.
There was a problem hiding this comment.
Ahh, that makes sense. I'll try that. Thanks :)
There was a problem hiding this comment.
Something to keep in mind, the code you'll send to the kernel will likely be specific to python. Try to keep the python semantics out of the internal request as far down as you can go. This will make it easier to add support for say C#/Julia/R in the future.
For example, you wouldn't want the sort column message from the UI to have any python specific stuff in it.
Codecov Report
@@ Coverage Diff @@
## main #5876 +/- ##
=====================================
- Coverage 72% 72% -1%
=====================================
Files 403 403
Lines 26971 26971
Branches 3941 3941
=====================================
- Hits 19432 19427 -5
+ Misses 5987 5911 -76
- Partials 1552 1633 +81
|
For #4585
package-lock.jsonhas been regenerated by runningnpm install(if dependencies have changed).