Fix PR review comments on cookie banner i18n

Address locale normalization for region-tagged values, guard language detection for non-DOM runtimes, validate DefaultLanguage on update, pass translated texts through the deactivation flow, handle slug collisions in migration, add organizations FK, fix consent migration from name-keyed to slug-keyed data, render all template placeholders in previews, and wrap helper text for i18n. Signed-off-by: Émile Ré <emile@getprobo.com>
2026-04-23 23:41:37 +04:00
parent fa7a9c96c1
commit 2b6f131f43
17 changed files with 89 additions and 55 deletions
--- a/apps/console/src/pages/organizations/cookie-banners/configuration/cookies/_components/EditCategoryForm.tsx
+++ b/apps/console/src/pages/organizations/cookie-banners/configuration/cookies/_components/EditCategoryForm.tsx
@@ -59,7 +59,10 @@ export function EditCategoryForm({
      />
      <div className="flex items-center gap-2">
        <Button
-          onClick={() => onSave(editName, editSlug, editDescription)}
+          onClick={() => {
+            if (!/^[a-z0-9]+(-[a-z0-9]+)*$/.test(editSlug)) return;
+            onSave(editName, editSlug, editDescription);
+          }}
          disabled={isUpdating}
        >
          {isUpdating ? __("Saving...") : __("Save")}
--- a/apps/console/src/pages/organizations/cookie-banners/configuration/translations/CookieBannerTranslationsPage.tsx
+++ b/apps/console/src/pages/organizations/cookie-banners/configuration/translations/CookieBannerTranslationsPage.tsx
@@ -68,11 +68,6 @@ export default function CookieBannerTranslationsPage({

  const banner = data.node;

-  const existingLanguages = useMemo(
-    () => banner.translations.map(t => t.language),
-    [banner.translations],
-  );
-
  const [selectedLanguage, setSelectedLanguage] = useState(
    () => banner.defaultLanguage,
  );
@@ -128,11 +123,7 @@ export default function CookieBannerTranslationsPage({
          value={selectedLanguage}
          onValueChange={setSelectedLanguage}
        >
-          {SUPPORTED_LANGUAGES.filter(
-            l =>
-              existingLanguages.includes(l.code)
-              || l.code === selectedLanguage,
-          ).map(l => (
+          {SUPPORTED_LANGUAGES.map(l => (
            <Option key={l.code} value={l.code}>
              {l.label}
              {l.code === banner.defaultLanguage ? ` (${__("default")})` : ""}
--- a/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/BannerPreview.tsx
+++ b/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/BannerPreview.tsx
@@ -28,7 +28,7 @@ function interpolateDescription(
  description: string,
  linkText: string,
 ): string {
-  return description.replace(
+  return description.replaceAll(
    "{{privacy_policy_link}}",
    linkText,
  );
@@ -79,22 +79,23 @@ export function BannerPreview({
        }}
      >
        {hasPlaceholder
-          ? (
-              <>
-                {descriptionParts[0]}
-                <a
-                  href="#"
-                  onClick={e => e.preventDefault()}
-                  style={{
-                    color: "var(--probo-accent, #1a1a1a)",
-                    textDecoration: "underline",
-                  }}
-                >
-                  {privacyPolicyLinkText}
-                </a>
-                {descriptionParts[1]}
-              </>
-            )
+          ? descriptionParts.map((part, i) => (
+              <span key={i}>
+                {part}
+                {i < descriptionParts.length - 1 && (
+                  <a
+                    href="#"
+                    onClick={e => e.preventDefault()}
+                    style={{
+                      color: "var(--probo-accent, #1a1a1a)",
+                      textDecoration: "underline",
+                    }}
+                  >
+                    {privacyPolicyLinkText}
+                  </a>
+                )}
+              </span>
+            ))
          : (
              interpolateDescription(bannerDescription, privacyPolicyLinkText)
            )}
--- a/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/PanelTranslationSection.tsx
+++ b/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/PanelTranslationSection.tsx
@@ -44,6 +44,12 @@ export function PanelTranslationSection({

  const visibleCategories = categories.filter(c => c.kind !== "UNCATEGORISED");

+  const translatedNecessaryName = (() => {
+    const necessaryCat = categories.find(c => c.kind === "NECESSARY");
+    if (!necessaryCat) return necessaryCategoryName;
+    return categoryTranslations?.[necessaryCat.id]?.name || necessaryCategoryName;
+  })();
+
  const previewCategories = categories.map((c) => {
    const translated = categoryTranslations?.[c.id];
    return {
@@ -75,7 +81,7 @@ export function PanelTranslationSection({
                <Field
                  label={__(TRANSLATION_LABELS.panel_description)}
                >
-                  <p className="text-xs text-txt-secondary mb-2">{"Use {{necessary_category}} to refer to the required cookies category name."}</p>
+                  <p className="text-xs text-txt-secondary mb-2">{__("Use {{necessary_category}} to refer to the required cookies category name.")}</p>
                  <Textarea {...field} rows={3} />
                </Field>
              )}
@@ -146,7 +152,7 @@ export function PanelTranslationSection({
            buttonRejectAll={buttonRejectAll}
            buttonSave={buttonSave}
            categories={previewCategories}
-            necessaryCategoryName={necessaryCategoryName}
+            necessaryCategoryName={translatedNecessaryName}
          />
        </div>
      </div>
--- a/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/PlaceholderPreview.tsx
+++ b/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/PlaceholderPreview.tsx
@@ -73,14 +73,13 @@ export function PlaceholderPreview({
      </span>
      <p style={{ margin: 0 }}>
        {hasPlaceholder
-          ? (
-              <>
-                {parts[0]}
-                <strong>{categoryName}</strong>
-                {parts[1]}
-              </>
-            )
-          : placeholderText.replace("{{category}}", categoryName)}
+          ? parts.map((part, i) => (
+              <span key={i}>
+                {part}
+                {i < parts.length - 1 && <strong>{categoryName}</strong>}
+              </span>
+            ))
+          : placeholderText}
      </p>
      <button
        type="button"
--- a/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/PlaceholderTranslationSection.tsx
+++ b/apps/console/src/pages/organizations/cookie-banners/configuration/translations/_components/PlaceholderTranslationSection.tsx
@@ -51,7 +51,7 @@ export function PlaceholderTranslationSection({
                <Field
                  label={__(TRANSLATION_LABELS.placeholder_text)}
                >
-                  <p className="text-xs text-txt-secondary mb-2">{"Use {{category}} to refer to the content category."}</p>
+                  <p className="text-xs text-txt-secondary mb-2">{__("Use {{category}} to refer to the content category.")}</p>
                  <Input {...field} />
                </Field>
              )}
--- a/packages/cookie-banner/src/activation.ts
+++ b/packages/cookie-banner/src/activation.ts
@@ -302,7 +302,7 @@ function deactivateScript(el: HTMLScriptElement): void {
  el.parentNode!.replaceChild(replacement, el);
 }

-function deactivateElement(el: Element, label?: string): void {
+function deactivateElement(el: Element, label?: string, texts?: BannerTexts): void {
  const category = el.getAttribute(ATTR_ACTIVATED);

  const src = el.getAttribute("src");
@@ -323,7 +323,7 @@ function deactivateElement(el: Element, label?: string): void {
  el.removeAttribute(ATTR_ACTIVATED);

  if (category && VISUAL_TAGS.has(el.tagName)) {
-    createPlaceholder(el, category, label);
+    createPlaceholder(el, category, label, texts);
  }
 }

@@ -332,6 +332,7 @@ export function deactivateElements(
  consentData: Record<string, boolean>,
  categoryCookies: Record<string, string[]>,
  categoryLabels: Record<string, string>,
+  texts?: BannerTexts,
 ): void {
  const elements = document.querySelectorAll(`[${ATTR_ACTIVATED}]`);
  const cookiesToRemove = new Set<string>();
@@ -345,7 +346,7 @@ export function deactivateElements(
    if (el instanceof HTMLScriptElement) {
      deactivateScript(el);
    } else {
-      deactivateElement(el, categoryLabels[category]);
+      deactivateElement(el, categoryLabels[category], texts);
    }

    const cookies = categoryCookies[category];
--- a/packages/cookie-banner/src/client.ts
+++ b/packages/cookie-banner/src/client.ts
@@ -250,7 +250,7 @@ export class CookieBannerClient {
    }

    const texts = this.config.texts;
-    deactivateElements(consentData, categoryCookies, categoryLabels);
+    deactivateElements(consentData, categoryCookies, categoryLabels, texts);
    activateElements(consentData);
    addPlaceholders(consentData, categoryLabels, texts);
    if (this.observer) {
--- a/packages/cookie-banner/src/components/cookie-banner-root.ts
+++ b/packages/cookie-banner/src/components/cookie-banner-root.ts
@@ -110,8 +110,8 @@ export class ProboCookieBannerRoot extends ProboElement implements ProboRootElem
    for (const cat of config.categories) {
      if (cat.kind === "NECESSARY") {
        draft[cat.slug] = true;
-      } else if (existing && cat.slug in existing) {
-        draft[cat.slug] = existing[cat.slug];
+      } else if (existing && (cat.slug in existing || cat.name in existing)) {
+        draft[cat.slug] = existing[cat.slug] ?? existing[cat.name];
      } else {
        draft[cat.slug] = config.consent_mode === "OPT_OUT";
      }
--- a/packages/cookie-banner/src/components/settings-button.ts
+++ b/packages/cookie-banner/src/components/settings-button.ts
@@ -29,6 +29,15 @@ export class ProboSettingsButton extends HTMLElement {
    return ["position", "aria-settings-label"];
  }

+  attributeChangedCallback(name: string, _oldValue: string | null, newValue: string | null): void {
+    if (name === "aria-settings-label") {
+      const btn = this.shadow.querySelector("button");
+      if (btn && newValue) {
+        btn.setAttribute("aria-label", newValue);
+      }
+    }
+  }
+
  private get position(): string {
    return this.getAttribute("position") ?? "bottom-left";
  }
--- a/packages/cookie-banner/src/i18n.ts
+++ b/packages/cookie-banner/src/i18n.ts
@@ -16,14 +16,20 @@ export interface BannerTexts {
  [key: string]: string;
 }

-export function detectLanguage(explicit?: string): string {
-  if (explicit) return explicit;
+function normalizeLocale(locale: string): string {
+  return locale.split("-")[0].toLowerCase();
+}

-  const htmlLang = document.documentElement.lang;
-  if (htmlLang) return htmlLang.split("-")[0];
+export function detectLanguage(explicit?: string): string {
+  if (explicit) return normalizeLocale(explicit);
+
+  if (typeof document !== "undefined" && document.documentElement) {
+    const htmlLang = document.documentElement.lang;
+    if (htmlLang) return normalizeLocale(htmlLang);
+  }

  if (typeof navigator !== "undefined" && navigator.language) {
-    return navigator.language.split("-")[0];
+    return normalizeLocale(navigator.language);
  }

  return "";
--- a/packages/cookie-banner/src/themed-banner/iife.ts
+++ b/packages/cookie-banner/src/themed-banner/iife.ts
@@ -40,7 +40,7 @@ if (script) {

      const lang = script.getAttribute("data-lang");
      if (lang) {
-        el.setAttribute("lang", lang);
+        el.setAttribute("lang", lang.split("-")[0].toLowerCase());
      }

      document.body.appendChild(el);
--- a/packages/cookie-banner/src/themed-banner/themed-banner.ts
+++ b/packages/cookie-banner/src/themed-banner/themed-banner.ts
@@ -170,7 +170,7 @@ export class ProboThemedBanner extends HTMLElement {

    this.shadow.querySelectorAll("[data-text]").forEach(el => {
      const key = el.getAttribute("data-text")!;
-      const raw = texts[key];
+      const raw = texts[key] ?? "";
      if (!raw) return;

      if (key === "banner_description") {
@@ -190,7 +190,7 @@ export class ProboThemedBanner extends HTMLElement {

    this.shadow.querySelectorAll("[data-aria-text]").forEach(el => {
      const key = el.getAttribute("data-aria-text")!;
-      const raw = texts[key];
+      const raw = texts[key] ?? el.getAttribute("aria-label") ?? "";
      if (raw) el.setAttribute("aria-label", raw);
    });

--- a/pkg/cookiebanner/defaults.go
+++ b/pkg/cookiebanner/defaults.go
@@ -16,6 +16,8 @@ package cookiebanner

 import "go.probo.inc/probo/pkg/coredata"

+var SupportedLanguages = []string{"en", "fr", "de", "es"}
+
 var defaultCategories = []struct {
 	Name        string
 	Slug        string
--- a/pkg/cookiebanner/service.go
+++ b/pkg/cookiebanner/service.go
@@ -177,6 +177,7 @@ func (r *UpdateCookieBannerRequest) Validate() error {
 	v.Check(r.PrivacyPolicyURL, "privacy_policy_url", validator.URL())
 	v.Check(r.ConsentExpiryDays, "consent_expiry_days", validator.Min(1))
 	v.Check(r.ConsentMode, "consent_mode", validator.OneOfSlice(coredata.CookieConsentModes()))
+	v.Check(r.DefaultLanguage, "default_language", validator.OneOfSlice(SupportedLanguages))

 	return v.Error()
 }
--- a/pkg/coredata/migrations/20260422T145821Z.sql
+++ b/pkg/coredata/migrations/20260422T145821Z.sql
@@ -18,7 +18,7 @@ ALTER TABLE cookie_banners ALTER COLUMN default_language DROP DEFAULT;
 CREATE TABLE cookie_banner_translations (
    id               TEXT PRIMARY KEY,
    tenant_id        TEXT NOT NULL,
-    organization_id  TEXT NOT NULL,
+    organization_id  TEXT NOT NULL REFERENCES organizations(id),
    cookie_banner_id TEXT NOT NULL REFERENCES cookie_banners(id) ON DELETE CASCADE,
    language         TEXT NOT NULL,
    translations     JSONB NOT NULL,
--- a/pkg/coredata/migrations/20260423T144457Z.sql
+++ b/pkg/coredata/migrations/20260423T144457Z.sql
@@ -16,7 +16,22 @@ ALTER TABLE cookie_categories
    ADD COLUMN slug TEXT NOT NULL DEFAULT '';

 UPDATE cookie_categories
-SET slug = LOWER(REGEXP_REPLACE(REGEXP_REPLACE(name, '[^a-zA-Z0-9]+', '-', 'g'), '^-|-$', '', 'g'));
+SET slug = COALESCE(
+    NULLIF(LOWER(REGEXP_REPLACE(REGEXP_REPLACE(name, '[^a-zA-Z0-9]+', '-', 'g'), '^-|-$', '', 'g')), ''),
+    'category-' || SUBSTR(id, 1, 8)
+);
+
+WITH dupes AS (
+    SELECT id,
+           cookie_banner_id,
+           slug,
+           ROW_NUMBER() OVER (PARTITION BY cookie_banner_id, slug ORDER BY created_at) AS rn
+    FROM cookie_categories
+)
+UPDATE cookie_categories
+SET slug = cookie_categories.slug || '-' || dupes.rn
+FROM dupes
+WHERE cookie_categories.id = dupes.id AND dupes.rn > 1;

 ALTER TABLE cookie_categories ALTER COLUMN slug DROP DEFAULT;