msyavuz commented on code in PR #37396:
URL: https://github.com/apache/superset/pull/37396#discussion_r3348107011
##########
superset/utils/core.py:
##########
@@ -1813,6 +1813,42 @@ def extract_dataframe_dtypes(
return generic_types
+def extract_display_labels(
Review Comment:
Does this handle __timestamp to "Time" mapping too?
##########
superset-frontend/packages/superset-ui-core/src/query/types/QueryResponse.ts:
##########
@@ -54,6 +54,10 @@ export interface ChartDataResponseResult {
* Name of each column, for retaining the order of the output columns.
*/
colnames: string[];
+ /**
+ * Labels of each column, for display purpose.
+ */
+ collabels: string[];
Review Comment:
I think this might be optional as we are already doing ensureIsArray at some
point right?
##########
superset-frontend/src/explore/components/DataTablesPane/types.ts:
##########
@@ -78,6 +76,7 @@ export interface TableControlsProps {
export interface QueryResultInterface {
colnames: string[];
+ collabels: string[];
Review Comment:
I think the confusion comes from SamplesPane ensureIsArray which concedes it
can be undefined right?
##########
superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx:
##########
@@ -37,7 +37,7 @@ export function useGridColumns(
.filter((column: string) => Object.keys(data[0]).includes(column))
.map((key, index) => {
const colType = coltypes?.[index];
- const headerLabel = columnDisplayNames?.[key] ?? key;
+ const headerLabel = collabels?.[index];
Review Comment:
Also collabels is aligned to the full colnames, but index here is the
post-.filter() index. When any column is filtered out, won't every subsequent
label shift by one and headers misalign? I think useTableColumns was reworked
to handle exactly this so maybe we can imnplement a similar thing here too?
##########
superset-frontend/src/explore/components/DataTableControl/index.tsx:
##########
@@ -236,13 +246,38 @@ const DataTableTemporalHeaderCell = ({
onClick={(e: React.MouseEvent<HTMLElement>) => e.stopPropagation()}
/>
</Popover>
- {displayLabel ?? columnName}
+ {columnLabel ?? columnName}
</span>
) : (
- <span>{displayLabel ?? columnName}</span>
+ <span>{columnLabel ?? columnName}</span>
);
};
+const DataTableHeaderCell = ({
+ columnName,
+ columnLabel,
+}: {
+ columnName: string;
+ columnLabel?: string;
+}) => {
+ // Use label if provided, otherwise use column name
+ // as header
+ const displayText = columnLabel || columnName;
Review Comment:
This has || while the temporal one above has ??. There is a subtle
difference when dealing with ''. Not sure if it's relevant but we can
consolidate on one
##########
superset-frontend/src/explore/components/DataTablesPane/components/useGridResultTable.tsx:
##########
@@ -37,7 +37,7 @@ export function useGridColumns(
.filter((column: string) => Object.keys(data[0]).includes(column))
.map((key, index) => {
const colType = coltypes?.[index];
- const headerLabel = columnDisplayNames?.[key] ?? key;
+ const headerLabel = collabels?.[index];
Review Comment:
This seems relevant
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
