codeant-ai-for-open-source[bot] commented on code in PR #37790: URL: https://github.com/apache/superset/pull/37790#discussion_r2827854841
########## superset-frontend/src/components/TranslationEditor/useTranslatableTitle.ts: ########## @@ -0,0 +1,159 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +import { type ReactNode, createElement, useCallback, useMemo, useState } from 'react'; +import { useSelector } from 'react-redux'; +import { t } from '@apache-superset/core'; +import { isFeatureEnabled, FeatureFlag } from '@superset-ui/core'; +import type { Translations } from 'src/types/Localization'; +import LocaleSwitcher from './LocaleSwitcher'; +import { DEFAULT_LOCALE_KEY } from './utils'; +import useAvailableLocales from './useAvailableLocales'; + +export interface UseTranslatableTitleParams { + /** Current default title. */ + title: string; + /** Full translations object for the entity. */ + translations?: Translations; + /** Translation field key (e.g., 'sliceNameOverride', 'text', 'slice_name'). */ + fieldName: string; + /** Save default title callback. */ + onSaveTitle: (value: string) => void; + /** Save translations callback. */ + onTranslationsChange: (translations: Translations) => void; + /** Human-readable field label for accessibility (e.g., 'Chart Name'). */ + fieldLabel: string; +} + +export interface UseTranslatableTitleResult { + /** Title to display in EditableTitle — localized value when in locale mode. */ + displayTitle: string; + /** Routes to onSaveTitle or onTranslationsChange based on activeLocale. */ + handleSave: (value: string) => void; + /** 'DEFAULT' or a locale code (e.g., 'de'). */ + activeLocale: string; + /** Switch the active locale. */ + setActiveLocale: (locale: string) => void; + /** Whether a specific locale (not DEFAULT) is active. */ + isLocaleMode: boolean; + /** Pre-rendered LocaleSwitcher component, or null when feature is off / no locales. */ + localeSwitcher: ReactNode | null; + /** Whether locale UI should be shown. */ + showLocale: boolean; + /** Placeholder for translation input when in locale mode. */ + placeholder: string; +} + +/** + * Hook for inline translatable title editors. + * + * Encapsulates locale state, display value resolution, save routing, + * and LocaleSwitcher rendering for components like SliceHeader, Tab, + * ExploreChartHeader, and Dashboard Header. + */ +export default function useTranslatableTitle({ + title, + translations, + fieldName, + onSaveTitle, + onTranslationsChange, + fieldLabel, +}: UseTranslatableTitleParams): UseTranslatableTitleResult { + const localizationEnabled = isFeatureEnabled( + FeatureFlag.EnableContentLocalization, + ); + const { allLocales, defaultLocale } = useAvailableLocales(); + const userLocale: string = useSelector( + (state: { common: { locale: string } }) => state.common?.locale ?? 'en', + ); + + const [activeLocale, setActiveLocale] = useState(DEFAULT_LOCALE_KEY); + + const isLocaleMode = activeLocale !== DEFAULT_LOCALE_KEY; + const showLocale = + localizationEnabled && allLocales.length > 0 && translations !== undefined; + Review Comment: **Suggestion:** The `showLocale` flag is only true when `translations` is not `undefined`, which means the locale switcher will never render for dashboards/charts whose translations field is initially undefined, preventing users from adding the first translation even when the feature flag and locales are configured. To fix this, don't gate `showLocale` on `translations` being defined and instead treat an undefined value as an empty translation map. [logic error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ New dashboards cannot add any locale-specific titles. - ❌ Chart and filter translations UI never appears initially. - ⚠️ Localization feature effectively unusable for fresh content. ``` </details> ```suggestion localizationEnabled && allLocales.length > 0; ``` <details> <summary><b>Steps of Reproduction ✅ </b></summary> ```mdx 1. Enable content localization so `localizationEnabled` is true and configure at least one locale, making `allLocales.length > 0` via `useAvailableLocales()` in `superset-frontend/src/components/TranslationEditor/useTranslatableTitle.ts:80`. 2. Open an editor UI (e.g., SliceHeader, Tab, ExploreChartHeader, or Dashboard Header as documented in this hook's comment at lines 64-68) for a newly created dashboard/chart whose `translations` field is absent/null in the API response, so its caller passes `translations` as `undefined` into `useTranslatableTitle(...)`. 3. When the hook runs, `showLocale` is computed at lines 88-90 as `localizationEnabled && allLocales.length > 0 && translations !== undefined`; since `translations` is `undefined` for this new entity, `showLocale` becomes `false` even though localization is enabled and locales exist. 4. In the same hook, the `localeSwitcher` memo at lines 124-137 sees `showLocale === false` and returns `null`, so the rendered header/editor UI never shows the `LocaleSwitcher` component, preventing the user from adding the first translation for that dashboard/chart. ``` </details> <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** superset-frontend/src/components/TranslationEditor/useTranslatableTitle.ts **Line:** 90:90 **Comment:** *Logic Error: The `showLocale` flag is only true when `translations` is not `undefined`, which means the locale switcher will never render for dashboards/charts whose translations field is initially undefined, preventing users from adding the first translation even when the feature flag and locales are configured. To fix this, don't gate `showLocale` on `translations` being defined and instead treat an undefined value as an empty translation map. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37790&comment_hash=d0f97bc480d8eda8446bfca4855ce42a568b965efaad208f6eeb2b56541f9580&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F37790&comment_hash=d0f97bc480d8eda8446bfca4855ce42a568b965efaad208f6eeb2b56541f9580&reaction=dislike'>👎</a> -- 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]
