You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by kg...@apache.org on 2026/06/02 14:19:20 UTC
(superset) branch feat/version-history-frontend created (now 7817412f75d)
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a change to branch feat/version-history-frontend
in repository https://gitbox.apache.org/repos/asf/superset.git
at 7817412f75d feat(versioning): figma audit pass 3 — timeline indicators + date format
This branch includes the following new commits:
new 03bbcc7a42a fix(chart): guard chartReducer against missing chart state
new 7817412f75d feat(versioning): figma audit pass 3 — timeline indicators + date format
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
(superset) 01/02: fix(chart): guard chartReducer against missing chart state
Posted by kg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch feat/version-history-frontend
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 03bbcc7a42a68bc1b8a2cbe36a1e7ac7c277d208
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Tue Jun 2 14:18:35 2026 +0000
fix(chart): guard chartReducer against missing chart state
CHART_UPDATE_STOPPED (and all other non-ADD_CHART handlers) reach for
fields off the existing ChartState. When the chart key is missing —
e.g. an in-flight stop dispatched after the chart was removed, or
before HYDRATE_DASHBOARD populated entries on a forked dashboard whose
position_json references slice ids that aren't associated with the new
dashboard — passing undefined to the handler crashes with
"Cannot read properties of undefined (reading 'queryController')".
Short-circuit the dispatch site so any non-ADD_CHART action targeting
a missing key is a no-op. Cheap, and protects every existing and
future handler at once.
Repro: Version-history side panel → row's three-dot menu →
"Open as new dashboard". Pre-fix: dashboard navigates and immediately
throws a TypeError in chartReducer. Post-fix: dashboard navigates,
charts that exist render normally, charts whose ids aren't associated
with the new dashboard show the standard "no chart definition"
placeholder instead of crashing the redux store.
Co-Authored-By: Claude Opus 4.7 <no...@anthropic.com>
---
.../src/components/Chart/chartReducer.ts | 8 +++++
.../src/components/Chart/chartReducers.test.ts | 37 ++++++++++++++++++++++
2 files changed, 45 insertions(+)
diff --git a/superset-frontend/src/components/Chart/chartReducer.ts b/superset-frontend/src/components/Chart/chartReducer.ts
index 66d99fabb25..af253bc0a45 100644
--- a/superset-frontend/src/components/Chart/chartReducer.ts
+++ b/superset-frontend/src/components/Chart/chartReducer.ts
@@ -229,6 +229,14 @@ export default function chartReducer(
}
if (action.type in actionHandlers) {
+ // ADD_CHART seeds a new entry; every other handler reads fields off the
+ // existing ChartState. If the key is missing (e.g. a CHART_UPDATE_*
+ // dispatched after the chart was removed, or before HYDRATE_DASHBOARD
+ // populated it on a forked dashboard), passing `undefined` to the
+ // handler crashes when it dereferences state.X. Drop the action instead.
+ if (action.type !== actions.ADD_CHART && !charts[action.key]) {
+ return charts;
+ }
return {
...charts,
[action.key]: actionHandlers[action.type](charts[action.key]),
diff --git a/superset-frontend/src/components/Chart/chartReducers.test.ts b/superset-frontend/src/components/Chart/chartReducers.test.ts
index d5b660dd810..5e213977530 100644
--- a/superset-frontend/src/components/Chart/chartReducers.test.ts
+++ b/superset-frontend/src/components/Chart/chartReducers.test.ts
@@ -67,6 +67,43 @@ describe('chart reducers', () => {
);
});
+ test('chartUpdateStopped is a no-op when the chart key is missing', () => {
+ const empty: Record<number, ChartState> = {};
+ const newState = chartReducer(empty, actions.chartUpdateStopped(999));
+ expect(newState).toBe(empty);
+ });
+
+ test('chartUpdateStarted is a no-op when the chart key is missing', () => {
+ const empty: Record<number, ChartState> = {};
+ const newState = chartReducer(
+ empty,
+ actions.chartUpdateStarted(new AbortController(), {}, 999),
+ );
+ expect(newState).toBe(empty);
+ });
+
+ test('chartUpdateSucceeded is a no-op when the chart key is missing', () => {
+ const empty: Record<number, ChartState> = {};
+ const newState = chartReducer(empty, actions.chartUpdateSucceeded([], 999));
+ expect(newState).toBe(empty);
+ });
+
+ test('chartUpdateFailed is a no-op when the chart key is missing', () => {
+ const empty: Record<number, ChartState> = {};
+ const newState = chartReducer(empty, actions.chartUpdateFailed([], 999));
+ expect(newState).toBe(empty);
+ });
+
+ test('addChart still seeds an entry when no prior state exists', () => {
+ const empty: Record<number, ChartState> = {};
+ const newState = chartReducer(
+ empty,
+ actions.addChart({ id: 42, formData: {} } as unknown as ChartState, 42),
+ );
+ expect(newState[42]).toBeDefined();
+ expect(newState[42].id).toEqual(42);
+ });
+
test('should update endtime on timeout', () => {
const newState = chartReducer(
charts,
(superset) 02/02: feat(versioning): figma audit pass 3 — timeline indicators + date format
Posted by kg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
kgabryje pushed a commit to branch feat/version-history-frontend
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 7817412f75d9a40c4233581963282703cbe894b1
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Tue Jun 2 14:18:59 2026 +0000
feat(versioning): figma audit pass 3 — timeline indicators + date format
- Current-version bucket header: filled green checkmark icon + sentence
case label + green text color. Bucket body keeps the light-green tint.
- Per-row indicator: filled green dot for the current row, hollow green
circle for past rows. Replaces the per-kind pen icon; the dot/circle
visually conveys "this is current vs past" at a glance and forms a
vertical timeline down the left edge of each bucket.
- Day-bucket header: calendar icon + mixed-case date with time-of-day
("Jun 1, 2026, 5:24 PM"). The displayed timestamp belongs to the
most-recent save in the day; rows are still grouped per calendar day.
Co-Authored-By: Claude Opus 4.7 <no...@anthropic.com>
---
.../versionHistory/components/VersionGroup.tsx | 23 +++++++++++++++++---
.../versionHistory/components/VersionItem.tsx | 24 +++++++++++++++++----
.../features/versionHistory/utils/groupActivity.ts | 25 +++++++++++++++++-----
3 files changed, 60 insertions(+), 12 deletions(-)
diff --git a/superset-frontend/src/features/versionHistory/components/VersionGroup.tsx b/superset-frontend/src/features/versionHistory/components/VersionGroup.tsx
index e35f5ebfd07..cf475018890 100644
--- a/superset-frontend/src/features/versionHistory/components/VersionGroup.tsx
+++ b/superset-frontend/src/features/versionHistory/components/VersionGroup.tsx
@@ -72,13 +72,14 @@ const VersionGroup = ({
width: 100%;
padding: ${theme.sizeUnit * 2}px ${theme.sizeUnit * 3}px
${theme.sizeUnit * 2}px ${theme.sizeUnit * 4}px;
- background: ${theme.colorBgLayout};
+ background: ${isCurrentBucket ? 'transparent' : theme.colorBgLayout};
border: none;
cursor: pointer;
font-size: ${theme.fontSizeSM}px;
font-weight: ${theme.fontWeightStrong};
- color: ${theme.colorTextSecondary};
- text-transform: uppercase;
+ color: ${isCurrentBucket
+ ? theme.colorSuccessText
+ : theme.colorTextSecondary};
`}
aria-expanded={open}
>
@@ -87,6 +88,22 @@ const VersionGroup = ({
) : (
<Icons.CaretRightOutlined iconSize="s" />
)}
+ {isCurrentBucket && (
+ <Icons.CheckCircleFilled
+ iconSize="s"
+ css={css`
+ color: ${theme.colorSuccess};
+ `}
+ />
+ )}
+ {!isCurrentBucket && (
+ <Icons.CalendarOutlined
+ iconSize="s"
+ css={css`
+ color: ${theme.colorIcon};
+ `}
+ />
+ )}
{label}
</button>
{open &&
diff --git a/superset-frontend/src/features/versionHistory/components/VersionItem.tsx b/superset-frontend/src/features/versionHistory/components/VersionItem.tsx
index 77e224567ce..b8d8803dfb4 100644
--- a/superset-frontend/src/features/versionHistory/components/VersionItem.tsx
+++ b/superset-frontend/src/features/versionHistory/components/VersionItem.tsx
@@ -29,7 +29,7 @@ import {
formatVersionUser,
isAiAuthor,
} from '../utils/formatVersionUser';
-import { iconForSave, totalChartImpact } from '../utils/changeKindIcon';
+import { totalChartImpact } from '../utils/changeKindIcon';
interface Props {
entityType: EntityType;
@@ -118,7 +118,6 @@ const VersionItem = ({
const supplied = save.changes.find(r => r.summary)?.summary;
return supplied || formatChangeTitle(save.changes);
}, [save.changes]);
- const leadingIcon = useMemo(() => iconForSave(save.changes), [save.changes]);
const impactCharts = useMemo(
() => totalChartImpact(save.changes),
[save.changes],
@@ -165,11 +164,28 @@ const VersionItem = ({
css={css`
display: inline-flex;
align-items: center;
- color: ${theme.colorIcon};
+ justify-content: center;
+ width: ${theme.sizeUnit * 3}px;
flex-shrink: 0;
+ padding-top: ${theme.sizeUnit / 2}px;
`}
>
- {leadingIcon}
+ {/* Timeline indicator: filled green dot for the current row,
+ hollow circle for past rows. Replaces the per-kind pen icon
+ so the user reads "this is current vs past" at a glance. */}
+ <span
+ data-test={
+ isCurrent ? 'version-item-dot-current' : 'version-item-dot-past'
+ }
+ css={css`
+ display: inline-block;
+ width: ${theme.sizeUnit * 2}px;
+ height: ${theme.sizeUnit * 2}px;
+ border-radius: 50%;
+ background: ${isCurrent ? theme.colorSuccess : 'transparent'};
+ border: 1px solid ${theme.colorSuccess};
+ `}
+ />
</div>
<div
css={css`
diff --git a/superset-frontend/src/features/versionHistory/utils/groupActivity.ts b/superset-frontend/src/features/versionHistory/utils/groupActivity.ts
index e5d2a165456..7d7a8d43d12 100644
--- a/superset-frontend/src/features/versionHistory/utils/groupActivity.ts
+++ b/superset-frontend/src/features/versionHistory/utils/groupActivity.ts
@@ -56,6 +56,12 @@ function startOfDay(d: Date): number {
// and a returning user can't tell whether "Today" is today-for-them or
// today-for-the-author. Both args kept for caller-symmetry with the
// earlier API + in case design wants relative labels back later.
+function bucketDayKey(iso: string): string {
+ // YYYY-MM-DD slice keeps rows from the same calendar day together
+ // regardless of the time-of-day suffix used in the user-visible label.
+ return iso.slice(0, 10);
+}
+
function bucketLabel(
iso: string,
_todayMs: number,
@@ -63,10 +69,12 @@ function bucketLabel(
): string {
try {
const lang = document.documentElement.lang || undefined;
- return new Date(iso).toLocaleDateString(lang, {
+ return new Date(iso).toLocaleString(lang, {
year: 'numeric',
- month: 'long',
+ month: 'short',
day: 'numeric',
+ hour: 'numeric',
+ minute: '2-digit',
});
} catch {
return iso.slice(0, 10);
@@ -170,13 +178,20 @@ export function groupActivity(records: ActivityRecord[]): ActivityDateBucket[] {
startIdx = 1;
}
+ // Bucket by calendar day, but render each bucket's header using the
+ // newest row's timestamp inside it — so the date+time the user sees is
+ // when the most recent save in that day happened, not midnight.
let active: ActivityDateBucket | null = null;
+ let activeDayKey: string | null = null;
for (let i = startIdx; i < rows.length; i += 1) {
const row = rows[i];
const iso = row.type === 'save' ? row.issued_at : row.record.issued_at;
- const label = bucketLabel(iso, today, yesterday);
- if (!active || active.label !== label) {
- active = { label, rows: [] };
+ const dayKey = bucketDayKey(iso);
+ if (!active || activeDayKey !== dayKey) {
+ // ``rows`` is newest-first within a day, so the first row we see
+ // for a new day-key is the one whose timestamp labels the bucket.
+ active = { label: bucketLabel(iso, today, yesterday), rows: [] };
+ activeDayKey = dayKey;
buckets.push(active);
}
active.rows.push(row);