-
Notifications
You must be signed in to change notification settings - Fork 61
feat: implement column sorting #1622
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
Conversation
|
I feel like the answer to this is obvious, but can we assume everyone with a REST connection has access to studio? And, is there a reason we shouldn't use a studio session for api calls/etc in the extension? I'm looking into column sorting, and However, you can provide sorting data for one or more column in studio like this: curl 'https://{studioUrl}/studio/sessions/{sessionId}/data/libraries/{library}/tables/{table}rows?start=0&limit=100&applyFormats=true&formatMissingValues=true' \
--data-raw '{
"distinct": false,
"filter": "",
"sorts": [
{ "columnName": "COLUMN_TO_SORT", "sortOrder": "descending", "sortPriority": 0 }
]
}' \This API call is what is used by the data viewer in studio. Is there a reason we shouldn't use this? |
|
Workbench goes through REST but no studio |
|
Looks like |
Agreed. We should use the sorting features available via compute/platform microservices where possible IMO. |
38c03da to
24e55aa
Compare
65f8813 to
5457e6a
Compare
|
Hey @kpatl1. I mentioned this in Teams, but wanted to get your thoughts on the changes to the |
|
I have also gone in and addressed comments in #1631 so hopefully that should be able to be merged soon |
|
suggest to change the PR title to be feat: ... |
|
|
||
| return { | ||
| rows: data.items, | ||
| count: data.count, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I tried it, there seems a problem. When get from a view, the count is undefined, thus the scrollbar doesn't show enough space.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh, good catch. I noticed there was some weirdness around loading larger tables while sorting, but thought it was mainly due to the fact that sorting took marginally longer than just displaying the table. Will take a look.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was resolved in 2a16789. This doesn't quite fix the issue with count being undefined...there wasn't a straightforward way to find that as it isn't returned by a view or by the createView statement.
However, this does resolve the issue enough (imo) as it stops us from indefinitely trying to load more data by inferring the total count.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So the user experience will be inconsistent. When I open a table (no sorting initially), I can see the scrollbar as if all rows are there. I can drag the scrollbar to a position and see empty cells and I know the content is being loaded. And later the content filled in.
Now when I add a sort, the scrollbar will change to like there're only 100 rows. When I scroll to the bottom, it's not obvious if it's really at end or loading.
It would be great if we can provide the count to ag grid. When the table opened initially, no sorting applied, we should already get the count. Can we cache the count?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I'd rather it not be inconsistent, either. I did consider caching things. That would fix the issue temporarily. However, once we incorporate filtering, I think we're going to run into the same issue and will have to manage this cache differently. Fwiw, it looks like scrollbars also get a little inconsistent in studio when we're applying both a sort and filter.
I'm inclined to not address this one until a user complains, unless @snlwih thinks it is worth exploring further before releasing?
|
Hey @scnwwu @smorrisj @danila-grobov . I'm wondering if any of you have guidance on how to proceed with the formatting issues? The actual sorting isn't the issue, but if I execute I thought I could correct the issue by applying formats in the query like... However, that returns improperly formatted results as well. Fwiw, I originally planned to use |
|
It looks like the "SAS Formats" only works well with $this.dataConnection.Execute("create view work.test as select * from sashelp.air order by air asc")
$objRecordSet = New-Object -comobject ADODB.Recordset
$objRecordSet.ActiveConnection = $this.dataConnection # This is needed to set the properties for sas formats.
$objRecordSet.Properties.Item("SAS Formats").Value = "_ALL_"
$objRecordSet.Open(
"work.test",
[System.Reflection.Missing]::Value, # Use the active connection
2, # adOpenDynamic
1, # adLockReadOnly
512 # adCmdTableDirect
)
$this.dataConnection.Execute("drop view work.test") |
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
- move useDataViewer - improve keyboard accessibility - improve localize Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
Signed-off-by: Scott Dover <[email protected]>
3a8e612 to
9f88adc
Compare
Summary:
This updates the data viewer code to support sorting columns.
On the backend, this is supported by passing a new
sortModelto ourgetRowsfunction. ITCconnections support this by creating an order by statement, but REST connections are a bit more
complicated. With a non-empty sort model, the general process for REST is to:
This is consistent with how studio sorts data.
On the front-end, a
ColumnMenuhas been introduced to facilitate adding and removing sortingcriteria.
Last, these changes required exposing l10n messages to the
ColumnMenucomponent. To do this, afew changes were made to
WebView:WebViewis now responsible for rendering it's contentWebView.rendermakes use of some new class methods to generate content:body: returns the html that should show up in the<body>section of a webviewscripts(optional): An array of relative script sources to load for the webviewstyles(optional): An array of relative style sources to load for the webviewl10nMessages(optional): A map of l10n translation messages to expose to React(NOTE: These messages can be read using the new
localizehelper)The changes to
WebViewshould make future webview panels work in a more consistent way.Testing:
to make sure sort indexing is maintained)
should be displayed to the left
to be near the right edge of the screen, and the child menu should be displayed to the left
esc