fix: address bugs found in sprint W-0.5 review

- Add appearance_editor.js script tag to base.templ (widget wasn't loading)
- Add hex color and gradient direction validation in UpdateTopbarStyle
- Add brandLogo length validation and include it in audit log
- Add destroy handler to appearance_editor widget (timer cleanup on unmount)
- Add user-facing error notifications via Chronicle.notify on API failures
- Add console.warn for malformed topbar-style JSON in widget init
- Add interface method docstrings for UpdateBranding/UpdateTopbarStyle
- Remove unused to-bl gradient direction from topbarInlineStyle

https://claude.ai/code/session_01QJLkgjQDu5qohzJKGV4hj9

Author: claude

internal/plugins/campaigns/handler.go

@@ -492,6 +492,7 @@ func (h *Handler) UpdateBrandingAPI(c echo.Context) error {
 
 	h.logAudit(c, cc.Campaign.ID, "campaign.branding.updated", map[string]any{
 		"brand_name": req.BrandName,
+		"brand_logo": req.BrandLogo,
 	})
 
 	return c.JSON(http.StatusOK, map[string]string{"status": "ok"})

internal/plugins/campaigns/service.go

@@ -51,7 +51,9 @@ type CampaignService interface {
 	// Backdrop and branding
 	UpdateBackdropPath(ctx context.Context, campaignID string, path *string) error
 	UpdateAccentColor(ctx context.Context, campaignID string, color string) error
+	// UpdateBranding sets the campaign's custom brand name and logo path.
 	UpdateBranding(ctx context.Context, campaignID, brandName, brandLogo string) error
+	// UpdateTopbarStyle sets the campaign's topbar visual customization.
 	UpdateTopbarStyle(ctx context.Context, campaignID string, style *TopbarStyle) error
 	UpdateDmGrants(ctx context.Context, campaignID string, userIDs []string) error
 
@@ -646,6 +648,9 @@ func (s *campaignService) UpdateBranding(ctx context.Context, campaignID, brandN
 	if len(brandName) > 40 {
 		return apperror.NewBadRequest("brand name must be 40 characters or fewer")
 	}
+	if len(brandLogo) > 255 {
+		return apperror.NewBadRequest("brand logo path too long")
+	}
 
 	campaign, err := s.repo.FindByID(ctx, campaignID)
 	if err != nil {
@@ -675,6 +680,21 @@ func (s *campaignService) UpdateTopbarStyle(ctx context.Context, campaignID stri
 		default:
 			return apperror.NewBadRequest("invalid topbar mode, expected solid, gradient, or image")
 		}
+		// Validate color values are valid hex (e.g. "#6366f1") or empty.
+		for _, color := range []string{style.Color, style.GradientFrom, style.GradientTo} {
+			if color != "" && !isValidHexColor(color) {
+				return apperror.NewBadRequest("invalid color format, expected #RRGGBB")
+			}
+		}
+		// Validate gradient direction.
+		if style.GradientDir != "" {
+			switch style.GradientDir {
+			case "to-r", "to-br", "to-b":
+				// ok
+			default:
+				return apperror.NewBadRequest("invalid gradient direction")
+			}
+		}
 	}
 
 	campaign, err := s.repo.FindByID(ctx, campaignID)
@@ -693,6 +713,19 @@ func (s *campaignService) UpdateTopbarStyle(ctx context.Context, campaignID stri
 	return s.repo.UpdateSettings(ctx, campaignID, string(settingsJSON))
 }
 
+// isValidHexColor checks that s is a 7-character hex color (#RRGGBB).
+func isValidHexColor(s string) bool {
+	if len(s) != 7 || s[0] != '#' {
+		return false
+	}
+	for _, c := range s[1:] {
+		if !((c >= '0' && c <= '9') || (c >= 'a' && c <= 'f') || (c >= 'A' && c <= 'F')) {
+			return false
+		}
+	}
+	return true
+}
+
 // UpdateDmGrants sets which users are granted dm_only content visibility.
 // Only campaign Owners may call this. The granted users can see dm_only
 // content but cannot create or toggle dm_only flags.

internal/templates/layouts/app.templ

@@ -969,8 +969,6 @@ func topbarInlineStyle(ctx context.Context) string {
 			dir = "to bottom right"
 		case "to-b":
 			dir = "to bottom"
-		case "to-bl":
-			dir = "to bottom left"
 		}
 		if style.GradientFrom != "" && style.GradientTo != "" {
 			return fmt.Sprintf("background: linear-gradient(%s, %s, %s); border-color: transparent;", dir, style.GradientFrom, style.GradientTo)

internal/templates/layouts/base.templ

@@ -152,6 +152,9 @@ templ Base(title string) {
 		<!-- Entity posts (sub-notes) -->
 		<script src="/static/js/widgets/entity_posts.js" defer></script>
 
+		<!-- Appearance editor widget (campaign visual customization) -->
+		<script src="/static/js/widgets/appearance_editor.js" defer></script>
+
 		<!-- Content template picker (entity create form + editor insert) -->
 		<script src="/static/js/template_picker.js" defer></script>
 

static/js/widgets/appearance_editor.js

@@ -25,6 +25,15 @@
   };
 
   Chronicle.register('appearance-editor', {
+    destroy: function (el) {
+      // Clear any pending debounce timers to prevent saves after unmount.
+      if (el._appearanceBrandTimer) {
+        clearTimeout(el._appearanceBrandTimer);
+      }
+      if (el._appearanceTopbarTimer) {
+        clearTimeout(el._appearanceTopbarTimer);
+      }
+    },
     init: function (el, config) {
       var campaignId = config.campaignId;
       var csrfToken = config.csrf;
@@ -41,7 +50,7 @@
           topbarStyle = parsed;
         }
       } catch (e) {
-        // Ignore parse errors, use defaults.
+        console.warn('[appearance-editor] Invalid topbar-style JSON, using defaults');
       }
 
       // DOM references.
@@ -57,9 +66,9 @@
       var gradToInput = el.querySelector('#appearance-topbar-gradient-to');
       var gradDirSelect = el.querySelector('#appearance-topbar-gradient-dir');
 
-      // Timers for debounced saves.
-      var brandTimer = null;
-      var topbarTimer = null;
+      // Timers for debounced saves (stored on element for destroy cleanup).
+      el._appearanceBrandTimer = null;
+      el._appearanceTopbarTimer = null;
 
       // --- Initialization ---
 
@@ -90,8 +99,8 @@
             previewBrand.textContent = brandInput.value || brandInput.placeholder;
           }
           // Debounced save.
-          clearTimeout(brandTimer);
-          brandTimer = setTimeout(function () {
+          clearTimeout(el._appearanceBrandTimer);
+          el._appearanceBrandTimer = setTimeout(function () {
             saveBranding(brandInput.value);
           }, DEBOUNCE_MS);
         });
@@ -105,7 +114,7 @@
               previewBrand.textContent = brandInput.placeholder;
             }
           }
-          clearTimeout(brandTimer);
+          clearTimeout(el._appearanceBrandTimer);
           saveBranding('');
         });
       }
@@ -208,8 +217,8 @@
        * Debounced save for topbar style.
        */
       function debouncedSaveTopbar() {
-        clearTimeout(topbarTimer);
-        topbarTimer = setTimeout(function () {
+        clearTimeout(el._appearanceTopbarTimer);
+        el._appearanceTopbarTimer = setTimeout(function () {
           saveTopbarStyle();
         }, DEBOUNCE_MS);
       }
@@ -222,8 +231,10 @@
           method: 'PUT',
           body: { brand_name: brandName },
           csrfToken: csrfToken
-        }).catch(function (err) {
-          console.error('[appearance-editor] Failed to save branding:', err);
+        }).then(function (res) {
+          if (!res.ok) { Chronicle.notify('Failed to save brand name', 'error'); }
+        }).catch(function () {
+          Chronicle.notify('Failed to save brand name', 'error');
         });
       }
 
@@ -243,8 +254,10 @@
           method: 'PUT',
           body: body,
           csrfToken: csrfToken
-        }).catch(function (err) {
-          console.error('[appearance-editor] Failed to save topbar style:', err);
+        }).then(function (res) {
+          if (!res.ok) { Chronicle.notify('Failed to save topbar style', 'error'); }
+        }).catch(function () {
+          Chronicle.notify('Failed to save topbar style', 'error');
         });
       }
     }