Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 25 additions & 4 deletions App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
sanitizeFacetSelections,
} from './src/utils/viewState';
import type { ViewState } from './src/utils/viewState';
import { computeSampleDefaults } from './src/utils/sampleDefaults';
import type { PCAVarianceEntry } from './components/ControlPanel';
import { clampTableHeight, computeDragHeight, isTableVisible, capTableRows } from './src/utils/tableLayout';

Expand Down Expand Up @@ -142,7 +143,9 @@ const App: React.FC = () => {

// Apply a completed parse result to app state. `sourceUrl` is the URL the
// CSV text was fetched from (null for local uploads / history / sample).
const applyParseResult = useCallback((result: Papa.ParseResult<DataPoint>, sourceUrl: string | null = null) => {
// `isSample` marks loads of the bundled sample dataset, which get colorful
// demo defaults (see src/utils/sampleDefaults.ts).
const applyParseResult = useCallback((result: Papa.ParseResult<DataPoint>, sourceUrl: string | null = null, isSample = false) => {
if (result.errors.length > 0) {
console.error("CSV Parsing errors:", result.errors);
alert("Error parsing CSV file. Check console for details.");
Expand Down Expand Up @@ -227,6 +230,24 @@ const App: React.FC = () => {
setShowColumnGroups(false);
setColumnGroups(new Map());

// First-impression demo defaults for the bundled sample dataset: color by
// species, tint cell borders by |r|, show regression lines. Pure decision
// logic lives in sampleDefaults.ts (PLAN.md guarantee (a)). A pending
// shared view wins outright — computeSampleDefaults returns null then and
// the #view= block below applies the sharer's settings instead. These are
// initial values only; every one of them remains a normal user toggle.
const sampleDefaults = computeSampleDefaults({
isSample,
hasPendingViewState: pendingViewRef.current !== null,
stringColumns: allStringCols,
});
if (sampleDefaults) {
setColorMode(sampleDefaults.colorMode);
setCategoryColorColumn(sampleDefaults.categoryColorColumn);
setTintCellBorders(sampleDefaults.tintCellBorders);
setShowRegressionLine(sampleDefaults.showRegressionLine);
}
Comment on lines +244 to +249

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reset sample-only settings on subsequent non-sample loads.

When computeSampleDefaults returns null for an upload/history/URL load, the prior sample's enabled tint and regression settings remain active; category mode also remains active if the next dataset contains species. Explicitly restore these sample-controlled settings to neutral for non-sample loads, then let pending #view= state override them.

Proposed fix
     if (sampleDefaults) {
       setColorMode(sampleDefaults.colorMode);
       setCategoryColorColumn(sampleDefaults.categoryColorColumn);
       setTintCellBorders(sampleDefaults.tintCellBorders);
       setShowRegressionLine(sampleDefaults.showRegressionLine);
+    } else if (!isSample) {
+      setColorMode('none');
+      setCategoryColorColumn(null);
+      setTintCellBorders(false);
+      setShowRegressionLine(false);
     }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (sampleDefaults) {
setColorMode(sampleDefaults.colorMode);
setCategoryColorColumn(sampleDefaults.categoryColorColumn);
setTintCellBorders(sampleDefaults.tintCellBorders);
setShowRegressionLine(sampleDefaults.showRegressionLine);
}
if (sampleDefaults) {
setColorMode(sampleDefaults.colorMode);
setCategoryColorColumn(sampleDefaults.categoryColorColumn);
setTintCellBorders(sampleDefaults.tintCellBorders);
setShowRegressionLine(sampleDefaults.showRegressionLine);
} else if (!isSample) {
setColorMode('none');
setCategoryColorColumn(null);
setTintCellBorders(false);
setShowRegressionLine(false);
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@App.tsx` around lines 244 - 249, Update the settings initialization around
the sampleDefaults handling in App so that when computeSampleDefaults returns
null, reset the sample-controlled color mode, category color column, tint cell
borders, and regression line to their neutral defaults. Preserve the existing
sample-default assignments when present, and apply any pending `#view`= state
after these resets so it can override them.


// Issue #43: a pending shared view (parsed from #view= at mount, or
// waiting for a manual load of a locally-shared file) overrides the
// defaults just set. This deliberately runs AFTER the standard resets
Expand Down Expand Up @@ -298,7 +319,7 @@ const App: React.FC = () => {
}
}, []);

const handleDataLoaded = useCallback((csvText: string, sourceUrl: string | null = null) => {
const handleDataLoaded = useCallback((csvText: string, sourceUrl: string | null = null, isSample = false) => {
// A new load supersedes any in-flight remote fetch or worker parse.
const loadId = ++loadRequestIdRef.current;
setIsUrlLoading(false);
Expand Down Expand Up @@ -326,7 +347,7 @@ const App: React.FC = () => {
// sourceUrl travels with the parse it belongs to (and behind the
// same request-id guard), so a slower, superseded load can never
// clobber a newer one's source.
applyParseResult(result, sourceUrl);
applyParseResult(result, sourceUrl, isSample);
},
});
});
Expand All @@ -343,7 +364,7 @@ const App: React.FC = () => {
.then(csvText => {
// Ignore this response if a newer load started in the meantime
if (loadRequestIdRef.current !== requestId) return;
handleDataLoaded(csvText);
handleDataLoaded(csvText, null, true);
})
.catch(error => {
console.error('Error loading sample data:', error);
Expand Down
100 changes: 100 additions & 0 deletions src/test/sampleDefaults.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
import { describe, it, expect } from 'vitest';
import {
computeSampleDefaults,
SAMPLE_CATEGORY_COLUMN,
} from '../utils/sampleDefaults';
import type { SampleDefaultsContext } from '../utils/sampleDefaults';

// The iris sample's string columns as detected by detectColumnTypes.
const IRIS_STRING_COLUMNS = ['species'];

const sampleContext = (overrides: Partial<SampleDefaultsContext> = {}): SampleDefaultsContext => ({
isSample: true,
hasPendingViewState: false,
stringColumns: IRIS_STRING_COLUMNS,
...overrides,
});

describe('computeSampleDefaults', () => {
it('exports the expected sample category column', () => {
expect(SAMPLE_CATEGORY_COLUMN).toBe('species');
});

describe('sample dataset (normal case)', () => {
it('returns the full colorful defaults for the iris sample', () => {
expect(computeSampleDefaults(sampleContext())).toEqual({
colorMode: 'category',
categoryColorColumn: 'species',
tintCellBorders: true,
showRegressionLine: true,
});
});

it('finds species among multiple string columns regardless of position', () => {
const result = computeSampleDefaults(
sampleContext({ stringColumns: ['label', 'species', 'notes'] })
);
expect(result?.colorMode).toBe('category');
expect(result?.categoryColorColumn).toBe('species');
});
});

describe('non-sample loads keep neutral defaults', () => {
it('returns null for uploaded / history / URL data even with a species column', () => {
expect(computeSampleDefaults(sampleContext({ isSample: false }))).toBeNull();
});

it('returns null for non-sample data with no string columns', () => {
expect(
computeSampleDefaults(sampleContext({ isSample: false, stringColumns: [] }))
).toBeNull();
});
});

describe('pending shared view state wins', () => {
it('returns null when a #view= state is pending, even for the sample', () => {
expect(
computeSampleDefaults(sampleContext({ hasPendingViewState: true }))
).toBeNull();
});

it('returns null when both non-sample and pending view apply', () => {
expect(
computeSampleDefaults(sampleContext({ isSample: false, hasPendingViewState: true }))
).toBeNull();
});
});

describe('sample without a species column (edge case)', () => {
it('still applies border tint and regression lines but not category coloring', () => {
expect(computeSampleDefaults(sampleContext({ stringColumns: [] }))).toEqual({
colorMode: 'none',
categoryColorColumn: null,
tintCellBorders: true,
showRegressionLine: true,
});
});

it('does not fall back to a different string column', () => {
const result = computeSampleDefaults(
sampleContext({ stringColumns: ['variety', 'label'] })
);
expect(result?.colorMode).toBe('none');
expect(result?.categoryColorColumn).toBeNull();
});

it('matches by exact name, not case-insensitively', () => {
const result = computeSampleDefaults(sampleContext({ stringColumns: ['Species'] }));
expect(result?.categoryColorColumn).toBeNull();
expect(result?.colorMode).toBe('none');
});
});

it('is pure: does not mutate the context it is given', () => {
const context = sampleContext();
const stringColumnsBefore = [...context.stringColumns];
computeSampleDefaults(context);
expect(context).toEqual(sampleContext());
expect(context.stringColumns).toEqual(stringColumnsBefore);
});
});
62 changes: 62 additions & 0 deletions src/utils/sampleDefaults.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import type { ColorMode } from '../../types';

/**
* Demo defaults for the bundled sample dataset (iris).
*
* The sample data is most people's first impression of the app, so instead of
* the neutral monochrome defaults it ships with the colorful features turned
* on: category coloring by species, |r|-tinted cell borders, and per-cell
* regression lines. These are ordinary initial values — the user can toggle
* each one off afterwards exactly like normal.
*
* The decision of WHETHER (and with what values) to apply those defaults is
* kept here as a pure function so it can be tested exhaustively
* (PLAN.md regression guarantee (a)).
*/

/** The sample dataset's category column used for color-by. */
export const SAMPLE_CATEGORY_COLUMN = 'species';

export interface SampleDefaultsContext {
/** True only when the load came from the bundled sample dataset
* (auto-load on first visit or the "Load Sample Data" button). */
isSample: boolean;
/** True when a shared #view= state is pending and about to be applied —
* the share link's settings must win over the demo defaults. */
hasPendingViewState: boolean;
/** String (categorical) columns detected in the loaded data. */
stringColumns: readonly string[];
}

export interface SampleDefaults {
colorMode: ColorMode;
categoryColorColumn: string | null;
tintCellBorders: boolean;
showRegressionLine: boolean;
}

/**
* Decide the initial demo defaults for a completed data load.
*
* Returns the defaults to apply, or null when the load must keep the current
* neutral behavior (non-sample data, or a pending shared view that overrides
* everything). If the sample somehow lacks its category column, the
* non-categorical extras (border tint, regression lines) still apply but
* color mode is left at 'none'.
*
* Note: tinting cell borders does not require the correlation badges —
* ScatterPlotMatrix computes correlations whenever either flag is on
* (`correlationActive = showCorrelation || tintCellBorders`).
*/
export function computeSampleDefaults(context: SampleDefaultsContext): SampleDefaults | null {
if (!context.isSample) return null;
if (context.hasPendingViewState) return null;

const hasCategoryColumn = context.stringColumns.includes(SAMPLE_CATEGORY_COLUMN);
return {
colorMode: hasCategoryColumn ? 'category' : 'none',
categoryColorColumn: hasCategoryColumn ? SAMPLE_CATEGORY_COLUMN : null,
tintCellBorders: true,
showRegressionLine: true,
};
}
Loading