feat: default sort config for tables in canvas#9648
Conversation
nishantmonu51
added
Type:Feature
|
@codex: review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c02a354952
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| let defaultSorting: SortingState; | ||
| $: defaultSorting = tableSpec.default_sort | ||
| ? [{ id: tableSpec.default_sort.id, desc: tableSpec.default_sort.desc }] |
There was a problem hiding this comment.
When a saved default sort points at a field that is no longer part of the current table (for example, set the default on sales, then remove sales from measures/columns or change the metrics view), this always rehydrates that stale id into pivotState.sorting. For nested pivots, getSortForAccessor only treats ids in the current measureNames or row dimension as stable; a stale stable id falls through to nested-accessor parsing and can index columnDimensionAxes[undefined][NaN], crashing the table, while flat tables send a sort for a non-selected field. Please clear or validate default_sort against the current fields before applying it.
Useful? React with 👍 / 👎.
| function makeDefault() { | ||
| if (!activeSort) return; | ||
| component.updateProperty("default_sort", { | ||
| id: activeSort.id, |
There was a problem hiding this comment.
For nested pivot leaf columns, activeSort.id is an accessor like c0v2m0, where v2 is only the current index in columnDimensionAxes. Saving that raw id as a default means the same YAML can sort by a different column value whenever filters, time range, or data change the column-axis ordering (or by undefined if the index disappears); the human label stored here is not used when applying the default. Store stable column-dimension values or invalidate nested defaults when axes change instead of persisting the positional accessor.
Useful? React with 👍 / 👎.
| pivotState.update((state) => ({ | ||
| ...state, | ||
| sorting: [], | ||
| sorting: defaultSorting, |
There was a problem hiding this comment.
Wont this override the sorting if there is a refetch? How about only setting it for the 1st time spec loaded? While this might not happen in cloud preview now, might be good to keep it correct.
| {/if} | ||
|
|
||
| <div class="flex items-center justify-between"> | ||
| <Button type="text" disabled={!canMakeDefault} onclick={makeDefault}> |
There was a problem hiding this comment.
Other inputs show the exact state of canvas in sidebar right? So this feels like it is breaking the norm. It also reads weird with Default sort header, sort pills but a set default button below it.
I know we might need some ephemeral sort state for user to look at before committing, so we might need a different location. Check the default filters with a button outside the sidebar for some inspiration.
3 participants
Adds support for default sort order configuration for pivot and flat tables in canvas dashboard
Checklist: