Data Explorer: Visually distinguish column names with leading/trailin…

…g whitespace or empty string (#5653) Aims to address #3084, #3089. Related to #5652, we also cannot distinguish empty strings or strings with leading whitespace in column names both in the data grid headers and the summary panel. This PR applies the same approach we used for data cells to format column names in the grid headers and the summary pane.
posit-dev · Jan 3, 2025 · 0841d7b · 0841d7b
1 parent 327cba2
commit 0841d7b
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 21 deletions.
diff --git a/src/vs/workbench/browser/positronDataGrid/components/dataGridColumnHeader.css b/src/vs/workbench/browser/positronDataGrid/components/dataGridColumnHeader.css
@@ -84,6 +84,14 @@
 	font-weight: var(--positron-data-grid-column-header-title-font-weight);
 }
 
+.data-grid-column-header
+.content
+.title-description
+.title
+.whitespace {
+	opacity: 50%;
+}
+
 .data-grid-column-header
 .content
 .title-description

diff --git a/src/vs/workbench/browser/positronDataGrid/components/dataGridColumnHeader.tsx b/src/vs/workbench/browser/positronDataGrid/components/dataGridColumnHeader.tsx
@@ -17,6 +17,7 @@ import { selectionType } from '../utilities/mouseUtilities.js';
 import { VerticalSplitter } from '../../../../base/browser/ui/positronComponents/splitters/verticalSplitter.js';
 import { ColumnSelectionState } from '../classes/dataGridInstance.js';
 import { usePositronDataGridContext } from '../positronDataGridContext.js';
+import { renderLeadingTrailingWhitespace } from '../../../services/positronDataExplorer/browser/components/tableDataCell.js';
 
 /**
  * Constants.
@@ -103,6 +104,8 @@ export const DataGridColumnHeader = (props: DataGridColumnHeaderProps) => {
 	// Determine whether the column is selected.
 	const selected = (columnSelectionState & ColumnSelectionState.Selected) !== 0;
 
+	const renderedColumn = renderLeadingTrailingWhitespace(props.column?.name);
+
 	// Render.
 	return (
 		<div
@@ -137,7 +140,7 @@ export const DataGridColumnHeader = (props: DataGridColumnHeaderProps) => {
 				}}
 			>
 				<div className='title-description'>
-					<div className='title'>{props.column?.name}</div>
+					<div className='title'>{renderedColumn}</div>
 					{props.column?.description &&
 						<div className='description'>{props.column.description}</div>
 					}

diff --git a/src/vs/workbench/services/positronDataExplorer/browser/components/columnSummaryCell.css b/src/vs/workbench/services/positronDataExplorer/browser/components/columnSummaryCell.css
@@ -91,6 +91,10 @@
 	grid-column: title / sparkline;
 }
 
+.data-grid-row-cell .content .column-summary .basic-info .column-name .whitespace {
+	opacity: 50%;
+}
+
 /* column-sparkline */
 
 .data-grid-row-cell .content .column-summary .basic-info .column-sparkline {

diff --git a/src/vs/workbench/services/positronDataExplorer/browser/components/columnSummaryCell.tsx b/src/vs/workbench/services/positronDataExplorer/browser/components/columnSummaryCell.tsx
@@ -24,6 +24,7 @@ import { ColumnProfileDatetime } from './columnProfileDatetime.js';
 import { TableSummaryDataGridInstance } from '../tableSummaryDataGridInstance.js';
 import { ColumnDisplayType, ColumnProfileType, ColumnSchema } from '../../../languageRuntime/common/positronDataExplorerComm.js';
 import { dataExplorerExperimentalFeatureEnabled } from '../../common/positronDataExplorerExperimentalConfig.js';
+import { renderLeadingTrailingWhitespace } from './tableDataCell.js';
 
 /**
  * Constants.
@@ -354,6 +355,8 @@ export const ColumnSummaryCell = (props: ColumnSummaryCellProps) => {
 			break;
 	}
 
+	const renderedColumn = renderLeadingTrailingWhitespace(props.columnSchema.column_name);
+
 	// Determine whether this is the cursor.
 	const cursor = props.columnIndex === props.instance.cursorRowIndex;
 
@@ -413,7 +416,7 @@ export const ColumnSummaryCell = (props: ColumnSummaryCellProps) => {
 					onMouseLeave={() => props.hoverService.hideHover()}
 				/>
 				<div className='column-name'>
-					{props.columnSchema.column_name}
+					{renderedColumn}
 				</div>
 				{!expanded && <ColumnSparkline />}
 				<ColumnNullPercent />

diff --git a/src/vs/workbench/services/positronDataExplorer/browser/components/tableDataCell.tsx b/src/vs/workbench/services/positronDataExplorer/browser/components/tableDataCell.tsx
@@ -23,33 +23,28 @@ interface TableDataCellProps {
 	dataCell: DataCell;
 }
 
-/**
- * TableDataCell component.
- * @param props A TableDataCellProps that contains the component properties.
- * @returns The rendered component.
- */
-export const TableDataCell = (props: TableDataCellProps) => {
-	const EMPTY_SPACE_SYMBOL = '\u00B7';
+export function renderLeadingTrailingWhitespace(text: string | undefined) {
+	const parts: (string | JSX.Element)[] = [];
 
-	let isSpecialValue = props.dataCell.kind !== DataCellKind.NON_NULL;
+	text = text ?? '';
 
-	// Render empty strings as special value
-	// Initialize rendered output parts
-	const parts: (string | JSX.Element)[] = [];
-	const formattedText = props.dataCell.formatted
-		.replace(/\r/g, '\\r')
-		.replace(/\n/g, '\\n');
+	if (text === '') {
+		// TODO: is this what we want?
+		return [<span className='whitespace'>{'<empty>'}</span>];
+	}
+
+	const EMPTY_SPACE_SYMBOL = '\u00B7';
 
 	// Handle text that is only whitespace
-	if (formattedText.trim() === '') {
+	if (text.trim() === '') {
 		parts.push(
 			<span className='whitespace'>
-				{EMPTY_SPACE_SYMBOL.repeat(formattedText.length)}
+				{EMPTY_SPACE_SYMBOL.repeat(text.length)}
 			</span>
 		);
 	} else {
 		// Handle leading whitespace
-		const leadingMatch = formattedText.match(/^\s+/);
+		const leadingMatch = text.match(/^\s+/);
 		if (leadingMatch) {
 			parts.push(
 				<span className='whitespace'>
@@ -59,11 +54,11 @@ export const TableDataCell = (props: TableDataCellProps) => {
 		}
 
 		// Add the main content
-		const mainContent = formattedText.trim();
+		const mainContent = text.trim();
 		parts.push(mainContent);
 
 		// Handle trailing whitespace
-		const trailingMatch = formattedText.match(/\s+$/);
+		const trailingMatch = text.match(/\s+$/);
 		if (trailingMatch) {
 			parts.push(
 				<span className='whitespace'>
@@ -72,6 +67,26 @@ export const TableDataCell = (props: TableDataCellProps) => {
 			);
 		}
 	}
+
+	return parts;
+}
+
+/**
+ * TableDataCell component.
+ * @param props A TableDataCellProps that contains the component properties.
+ * @returns The rendered component.
+ */
+export const TableDataCell = (props: TableDataCellProps) => {
+	// Render empty strings as special value
+	// Initialize rendered output parts
+	const formattedText = props.dataCell.formatted
+		.replace(/\r/g, '\\r')
+		.replace(/\n/g, '\\n');
+
+	const parts = renderLeadingTrailingWhitespace(formattedText);
+
+	let isSpecialValue = props.dataCell.kind !== DataCellKind.NON_NULL;
+
 	let renderedOutput = parts;
 	if (props.dataCell.kind === DataCellKind.NON_NULL && formattedText === '') {
 		isSpecialValue = true;