You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/04/27 08:53:43 UTC
(superset) 01/01: fix(glossary): address review feedback on tooltip glossary integration
This is an automated email from the ASF dual-hosted git repository.
rusackas pushed a commit to branch feat/tooltip-docs-integration
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 8750154a5eab3f005887cdb7f92184af85f04fd5
Author: Evan Rusackas <ev...@rusackas.com>
AuthorDate: Mon Apr 27 01:48:09 2026 -0700
fix(glossary): address review feedback on tooltip glossary integration
- Rename local identity `t()` in superset-ui-core/src/glossary/glossary.ts
to `noTranslate()` so it no longer visually shadows the imported i18n
`t` used elsewhere in the package (resolves Copilot review thread).
- Drop the no-op `t(SENTINEL_DESCRIPTION)` wrappers in 8 chart-control
files: the wrapped values are `[GLOSSARY]|topic|title` sentinels, not
translatable strings, so wrapping them in `t()` did nothing at runtime
and polluted pybabel extraction. Real translation already happens at
resolution time inside `resolveGlossaryString` via `term.getShort(t)`.
- Memoize `scrollToRow` with `useCallback` in docs/docs/glossary.mdx and
add it to the `useEffect` deps array (resolves Copilot review thread);
also wrap the `decodeURIComponent(window.location.hash)` call in
try/catch so malformed percent-encoding can't break the page render.
- Drop the bare `export default {}` from
superset-ui-core/src/glossary/index.ts; it shadowed the named
`glossary` export so default-imports silently received `{}`. All
in-repo consumers use named imports, so this is safe.
Out of scope (left for the original author):
- Unit tests for resolveGlossaryString, Tooltip branching, and the
GlossaryTerm.encode roundtrip.
- Making GLOSSARY_BASE_URL configurable for downstream deploys.
- Simplifying the parallel glossaryDefinition / GlossaryTerm /
GlossaryMap object models.
Co-Authored-By: Claude <no...@anthropic.com>
---
docs/docs/glossary.mdx | 17 ++++++++++-----
.../src/sections/advancedAnalytics.tsx | 2 +-
.../src/sections/timeComparison.tsx | 2 +-
.../src/shared-controls/dndControls.tsx | 6 +++---
.../src/shared-controls/sharedControls.tsx | 8 +++----
.../superset-ui-core/src/glossary/glossary.ts | 25 +++++++++++++---------
.../superset-ui-core/src/glossary/index.ts | 2 --
.../src/controlPanel.tsx | 2 +-
.../legacy-plugin-chart-rose/src/controlPanel.tsx | 2 +-
.../legacy-preset-chart-nvd3/src/NVD3Controls.tsx | 2 +-
.../src/explore/controlPanels/sections.tsx | 2 +-
11 files changed, 40 insertions(+), 30 deletions(-)
diff --git a/docs/docs/glossary.mdx b/docs/docs/glossary.mdx
index a77a1c337ee..dc81f0b7695 100644
--- a/docs/docs/glossary.mdx
+++ b/docs/docs/glossary.mdx
@@ -7,7 +7,7 @@ sidebar_position: 10
import { getAllGlossaryTopics } from '../../superset-frontend/packages/superset-ui-core/src/glossary';
import { Table, ConfigProvider, theme } from 'antd';
import { useColorMode } from '@docusaurus/theme-common';
-import { useEffect, useRef } from 'react';
+import { useCallback, useEffect, useRef } from 'react';
export const GlossaryStructure = [
{
@@ -28,7 +28,7 @@ export const GlossaryContent = () => {
const isDark = colorMode === 'dark';
const tableRefs = useRef({});
- const scrollToRow = (topic, rowKey) => {
+ const scrollToRow = useCallback((topic, rowKey) => {
const topicId = encodeURIComponent(topic);
const encRowKey = encodeURIComponent(rowKey);
const row = tableRefs.current[topicId]?.[encRowKey];
@@ -37,15 +37,22 @@ export const GlossaryContent = () => {
row.classList.add('table-row-highlight');
setTimeout(() => row.classList.remove('table-row-highlight'), 2000);
}
- };
+ }, []);
useEffect(() => {
- const hash = decodeURIComponent(window.location.hash.slice(1));
+ let hash = '';
+ try {
+ hash = decodeURIComponent(window.location.hash.slice(1));
+ } catch (e) {
+ // Malformed percent-encoding in the URL hash — silently skip the
+ // scroll-to-row behavior rather than letting the page render fail.
+ return;
+ }
if (!hash) return;
const [topic, term] = hash.split('__');
if (topic && term) scrollToRow(topic, hash);
- }, []);
+ }, [scrollToRow]);
return (
<div>
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx
index 87b82490d75..8b22ab43060 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/advancedAnalytics.tsx
@@ -126,7 +126,7 @@ export const advancedAnalyticsControls: ControlPanelSectionConfig = {
['156 weeks ago', t('156 weeks ago')],
['3 years ago', t('3 years ago')],
],
- description: t(TIME_SHIFT_DESCRIPTION),
+ description: TIME_SHIFT_DESCRIPTION,
},
},
],
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx
index da832147537..6ea564ee1de 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/sections/timeComparison.tsx
@@ -85,7 +85,7 @@ export const timeComparisonControls: ({
placeholder: t('Select or type a custom value...'),
label: t('Time shift'),
choices: showFullChoices ? fullChoices : reducedChoices,
- description: t(TIME_SHIFT_DESCRIPTION),
+ description: TIME_SHIFT_DESCRIPTION,
},
},
],
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
index 5b5ecb68906..19a36a5ec34 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/dndControls.tsx
@@ -84,7 +84,7 @@ export const dndGroupByControl: SharedControlConfig<
clearable: true,
default: [],
includeTime: false,
- description: t(DIMENSION_DESCRIPTION),
+ description: DIMENSION_DESCRIPTION,
optionRenderer: (c: ColumnMeta) => <ColumnOption showType column={c} />,
valueRenderer: (c: ColumnMeta) => <ColumnOption column={c} />,
valueKey: 'column_name',
@@ -182,7 +182,7 @@ export const dndAdhocMetricsControl: SharedControlConfig<
datasource,
datasourceType: datasource?.type,
}),
- description: t(METRIC_DESCRIPTION),
+ description: METRIC_DESCRIPTION,
};
export const dndAdhocMetricControl: typeof dndAdhocMetricsControl = {
@@ -222,7 +222,7 @@ export const dndSortByControl: SharedControlConfig<
type: 'DndMetricSelect',
label: t('Sort query by'),
default: null,
- description: t(SORT_DESCRIPTION),
+ description: SORT_DESCRIPTION,
mapStateToProps: ({ datasource }) => ({
columns: datasource?.columns || [],
savedMetrics: defineSavedMetrics(datasource),
diff --git a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
index e9485a0a478..740617906b6 100644
--- a/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
+++ b/superset-frontend/packages/superset-ui-chart-controls/src/shared-controls/sharedControls.tsx
@@ -239,7 +239,7 @@ const row_limit: SharedControlConfig<'SelectControl'> = {
],
default: 10000,
choices: formatSelectOptions(ROW_LIMIT_OPTIONS),
- description: t(ROW_LIMIT_DESCRIPTION),
+ description: ROW_LIMIT_DESCRIPTION,
};
const order_desc: SharedControlConfig<'CheckboxControl'> = {
@@ -252,7 +252,7 @@ const order_desc: SharedControlConfig<'CheckboxControl'> = {
visibility: ({ controls }) =>
Boolean(
controls?.timeseries_limit_metric.value &&
- !isEmpty(controls?.timeseries_limit_metric.value),
+ !isEmpty(controls?.timeseries_limit_metric.value),
),
};
@@ -264,7 +264,7 @@ const limit: SharedControlConfig<'SelectControl'> = {
validators: [legacyValidateInteger],
choices: formatSelectOptions(SERIES_LIMITS),
clearable: true,
- description: t(SERIES_DESCRIPTION),
+ description: SERIES_DESCRIPTION,
};
const series_limit: SharedControlConfig<'SelectControl'> = {
@@ -274,7 +274,7 @@ const series_limit: SharedControlConfig<'SelectControl'> = {
placeholder: t('None'),
validators: [legacyValidateInteger],
choices: formatSelectOptions(SERIES_LIMITS),
- description: t(SERIES_DESCRIPTION),
+ description: SERIES_DESCRIPTION,
};
const group_others_when_limit_reached: SharedControlConfig<'CheckboxControl'> =
diff --git a/superset-frontend/packages/superset-ui-core/src/glossary/glossary.ts b/superset-frontend/packages/superset-ui-core/src/glossary/glossary.ts
index 5c8c36b0a1a..0e019060225 100644
--- a/superset-frontend/packages/superset-ui-core/src/glossary/glossary.ts
+++ b/superset-frontend/packages/superset-ui-core/src/glossary/glossary.ts
@@ -31,8 +31,8 @@
* export const glossaryDefinition: GlossaryDefinition = {
* Query: {
* Row_Limit: {
- * short: t('Limits the number of rows...'),
- * extended: t('Additional details...'), // optional
+ * short: noTranslate('Limits the number of rows...'),
+ * extended: noTranslate('Additional details...'), // optional
* },
* },
* };
@@ -45,20 +45,20 @@
export const glossaryDefinition: GlossaryDefinition = {
Query: {
Dimension: {
- short: t(
+ short: noTranslate(
'Dimensions contain qualitative values such as names, dates, or geographical data. ' +
'Use dimensions to categorize, segment, and reveal the details in your data. ' +
'Dimensions affect the level of detail in the view.',
),
},
Metric: {
- short: t(
+ short: noTranslate(
'Select one or many metrics to display. ' +
'You can use an aggregation function on a column or write custom SQL to create a metric.',
),
},
Series: {
- short: t(
+ short: noTranslate(
'Limits the number of series that get displayed. ' +
'A joined subquery (or an extra phase where subqueries are not supported) is applied ' +
'to limit the number of series that get fetched and rendered. ' +
@@ -67,14 +67,14 @@ export const glossaryDefinition: GlossaryDefinition = {
),
},
Row_Limit: {
- short: t(
+ short: noTranslate(
'Limits the number of rows that get displayed. ' +
'This feature is useful when grouping by high cardinality column(s) ' +
'though does increase the query complexity and cost.',
),
},
Sort: {
- short: t(
+ short: noTranslate(
'Orders the query result that generates the source data for this chart. ' +
'If a series or row limit is reached, this determines what data are truncated. ' +
'If undefined, defaults to the first metric (where appropriate).',
@@ -83,7 +83,7 @@ export const glossaryDefinition: GlossaryDefinition = {
},
Advanced_Analytics: {
Time_Shift: {
- short: t(
+ short: noTranslate(
'Overlay results from a relative time period. ' +
'Expects relative time deltas in natural language (example: 24 hours, 7 days, ' +
'52 weeks, 365 days). Free text is supported. ' +
@@ -95,9 +95,14 @@ export const glossaryDefinition: GlossaryDefinition = {
};
/**
- * Use this identity translate function as some environments (docs) do not have i18n setup.
+ * Identity passthrough used in environments (such as the docs site) that do
+ * not have an i18n runtime. Translation of glossary strings is performed at
+ * resolution time by callers in app contexts that do have i18n available.
+ *
+ * Named `noTranslate` (rather than `t`) so it does not visually shadow the
+ * imported i18n `t` used elsewhere in this package.
*/
-function t(message: string): string {
+function noTranslate(message: string): string {
return message;
}
diff --git a/superset-frontend/packages/superset-ui-core/src/glossary/index.ts b/superset-frontend/packages/superset-ui-core/src/glossary/index.ts
index f23cd9c6c54..dfd179bf167 100644
--- a/superset-frontend/packages/superset-ui-core/src/glossary/index.ts
+++ b/superset-frontend/packages/superset-ui-core/src/glossary/index.ts
@@ -24,5 +24,3 @@ export {
getGlossaryTopic,
} from './glossaryUtils';
export { resolveGlossaryString } from './tooltipUtils';
-
-export default {};
diff --git a/superset-frontend/plugins/legacy-plugin-chart-partition/src/controlPanel.tsx b/superset-frontend/plugins/legacy-plugin-chart-partition/src/controlPanel.tsx
index d18e5657d73..93b55c82925 100644
--- a/superset-frontend/plugins/legacy-plugin-chart-partition/src/controlPanel.tsx
+++ b/superset-frontend/plugins/legacy-plugin-chart-partition/src/controlPanel.tsx
@@ -323,7 +323,7 @@ const config: ControlPanelConfig = {
['156 weeks', t('156 weeks')],
['3 years', t('3 years')],
],
- description: t(TIME_SHIFT_DESCRIPTION),
+ description: TIME_SHIFT_DESCRIPTION,
},
},
{
diff --git a/superset-frontend/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx b/superset-frontend/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx
index a08481e0672..33e49720511 100644
--- a/superset-frontend/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx
+++ b/superset-frontend/plugins/legacy-plugin-chart-rose/src/controlPanel.tsx
@@ -207,7 +207,7 @@ const config: ControlPanelConfig = {
['156 weeks', t('156 weeks')],
['3 years', t('3 years')],
],
- description: t(TIME_SHIFT_DESCRIPTION),
+ description: TIME_SHIFT_DESCRIPTION,
},
},
{
diff --git a/superset-frontend/plugins/legacy-preset-chart-nvd3/src/NVD3Controls.tsx b/superset-frontend/plugins/legacy-preset-chart-nvd3/src/NVD3Controls.tsx
index 0b834988e67..7c82439a836 100644
--- a/superset-frontend/plugins/legacy-preset-chart-nvd3/src/NVD3Controls.tsx
+++ b/superset-frontend/plugins/legacy-preset-chart-nvd3/src/NVD3Controls.tsx
@@ -476,7 +476,7 @@ export const timeSeriesSection: ControlPanelSectionConfig[] = [
['156 weeks', t('156 weeks')],
['3 years', t('3 years')],
],
- description: t(TIME_SHIFT_DESCRIPTION),
+ description: TIME_SHIFT_DESCRIPTION,
},
},
],
diff --git a/superset-frontend/src/explore/controlPanels/sections.tsx b/superset-frontend/src/explore/controlPanels/sections.tsx
index 13b43b98e43..85bb20c1ae4 100644
--- a/superset-frontend/src/explore/controlPanels/sections.tsx
+++ b/superset-frontend/src/explore/controlPanels/sections.tsx
@@ -207,7 +207,7 @@ export const NVD3TimeSeries: ControlPanelSectionConfig[] = [
['156 weeks', t('156 weeks')],
['3 years', t('3 years')],
],
- description: t(TIME_SHIFT_DESCRIPTION),
+ description: TIME_SHIFT_DESCRIPTION,
},
},
{