fix: address PR review comments for voice note renderers

Addressed all 10 PR review comments: 1. Accessibility: Add role="slider", tabIndex, keyboard navigation (Arrow keys, Home, End) and ARIA attributes to waveform/progress 2. Waveform parsing: Filter NaN values using getVoiceNoteMetadata from imeta utility with proper validation 3. Seek function: Use `duration > 0` instead of truthy check 4. JSDoc: Fix terminology to "Voice Message Reply" 5. ParentEventCard: Match NoteRenderer API with icon and tooltipText props for consistency 6. Math.max: Use reduce-based safeMax() to avoid stack overflow on large waveform arrays 7. CORS: Add crossOrigin="anonymous" to audio element 8. Audio cleanup: Pause audio in useEffect cleanup to prevent continued playback after unmount 9. Error reset: Add useEffect to reset error state when URL changes 10. Code reuse: Use getVoiceNoteMetadata from @/lib/imeta instead of custom parsing, extending ImetaEntry with waveform/duration
2026-04-10 15:36:53 +02:00 · 2025-12-21 11:52:24 +00:00
parent e1d24a0ba7
commit 9c30a179e9
2 changed files with 175 additions and 62 deletions
--- a/src/components/nostr/kinds/VoiceNoteRenderer.tsx
+++ b/src/components/nostr/kinds/VoiceNoteRenderer.tsx
@@ -1,4 +1,4 @@
-import { useState, useRef, useEffect } from "react";
+import { useState, useRef, useEffect, useCallback } from "react";
 import { kinds } from "nostr-tools";
 import { BaseEventContainer, type BaseEventProps } from "./BaseEventRenderer";
 import { Mic, Play, Pause, Reply } from "lucide-react";
@@ -10,7 +10,9 @@ import { InlineReplySkeleton } from "@/components/ui/skeleton";
 import { KindBadge } from "@/components/KindBadge";
 import { Button } from "@/components/ui/button";
 import { getEventDisplayTitle } from "@/lib/event-title";
+import { getVoiceNoteMetadata } from "@/lib/imeta";
 import type { NostrEvent } from "@/types/nostr";
+import type { LucideIcon } from "lucide-react";
 import {
  Tooltip,
  TooltipContent,
@@ -24,38 +26,6 @@ import { cn } from "@/lib/utils";
 const VOICE_MESSAGE_KIND = 1222;
 const VOICE_MESSAGE_COMMENT_KIND = 1244;

-/**
- * Parse voice note metadata from imeta tags (NIP-92)
- * Returns waveform data and duration if present
- */
-function parseVoiceNoteMetadata(event: NostrEvent): {
-  waveform?: number[];
-  duration?: number;
-} {
-  const result: { waveform?: number[]; duration?: number } = {};
-
-  for (const tag of event.tags) {
-    if (tag[0] !== "imeta") continue;
-
-    for (let i = 1; i < tag.length; i++) {
-      const parts = tag[i].split(" ");
-      if (parts.length < 2) continue;
-
-      const key = parts[0];
-      const value = parts.slice(1).join(" ");
-
-      if (key === "waveform") {
-        // Waveform is space-separated amplitude integers
-        result.waveform = value.split(" ").map((v) => parseInt(v, 10));
-      } else if (key === "duration") {
-        result.duration = parseFloat(value);
-      }
-    }
-  }
-
-  return result;
-}
-
 /**
 * Get audio URL from event content
 */
@@ -78,21 +48,32 @@ function formatDuration(seconds: number): string {
 }

 /**
- * Waveform visualization component
+ * Safe Math.max for potentially large arrays
+ * Uses reduce to avoid stack overflow from spread operator
+ */
+function safeMax(arr: number[], defaultValue = 0): number {
+  if (arr.length === 0) return defaultValue;
+  return arr.reduce((max, val) => (val > max ? val : max), arr[0]);
+}
+
+/**
+ * Waveform visualization component with accessibility support
 */
 function WaveformVisualization({
  waveform,
  progress,
-  onClick,
+  duration,
+  onSeek,
 }: {
  waveform: number[];
  progress: number; // 0-1
-  onClick?: (progress: number) => void;
+  duration: number;
+  onSeek?: (progress: number) => void;
 }) {
  const containerRef = useRef<HTMLDivElement>(null);

-  // Normalize waveform to 0-1 range
-  const maxAmplitude = Math.max(...waveform, 1);
+  // Normalize waveform to 0-1 range using safe max
+  const maxAmplitude = safeMax(waveform, 1);
  const normalizedWaveform = waveform.map((v) => v / maxAmplitude);

  // Limit to ~50 bars for display
@@ -101,21 +82,47 @@ function WaveformVisualization({
  const displayBars: number[] = [];
  for (let i = 0; i < waveform.length; i += step) {
    const chunk = normalizedWaveform.slice(i, i + step);
-    displayBars.push(Math.max(...chunk));
+    displayBars.push(safeMax(chunk, 0));
  }

  const handleClick = (e: React.MouseEvent<HTMLDivElement>) => {
-    if (!containerRef.current || !onClick) return;
+    if (!containerRef.current || !onSeek) return;
    const rect = containerRef.current.getBoundingClientRect();
    const clickProgress = (e.clientX - rect.left) / rect.width;
-    onClick(Math.max(0, Math.min(1, clickProgress)));
+    onSeek(Math.max(0, Math.min(1, clickProgress)));
+  };
+
+  const handleKeyDown = (e: React.KeyboardEvent) => {
+    if (!onSeek) return;
+    const step = 0.05; // 5% step
+    if (e.key === "ArrowLeft") {
+      e.preventDefault();
+      onSeek(Math.max(0, progress - step));
+    } else if (e.key === "ArrowRight") {
+      e.preventDefault();
+      onSeek(Math.min(1, progress + step));
+    } else if (e.key === "Home") {
+      e.preventDefault();
+      onSeek(0);
+    } else if (e.key === "End") {
+      e.preventDefault();
+      onSeek(1);
+    }
  };

  return (
    <div
      ref={containerRef}
-      className="flex items-center gap-[2px] h-8 cursor-pointer flex-1"
+      role="slider"
+      tabIndex={0}
+      aria-label="Audio progress"
+      aria-valuemin={0}
+      aria-valuemax={duration}
+      aria-valuenow={progress * duration}
+      aria-valuetext={`${formatDuration(progress * duration)} of ${formatDuration(duration)}`}
+      className="flex items-center gap-[2px] h-8 cursor-pointer flex-1 focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 rounded"
      onClick={handleClick}
+      onKeyDown={handleKeyDown}
    >
      {displayBars.map((amplitude, i) => {
        const barProgress = i / displayBars.length;
@@ -138,29 +145,57 @@ function WaveformVisualization({
 }

 /**
- * Simple progress bar fallback when no waveform is available
+ * Simple progress bar fallback with accessibility support
 */
 function SimpleProgressBar({
  progress,
-  onClick,
+  duration,
+  onSeek,
 }: {
  progress: number;
-  onClick?: (progress: number) => void;
+  duration: number;
+  onSeek?: (progress: number) => void;
 }) {
  const containerRef = useRef<HTMLDivElement>(null);

  const handleClick = (e: React.MouseEvent<HTMLDivElement>) => {
-    if (!containerRef.current || !onClick) return;
+    if (!containerRef.current || !onSeek) return;
    const rect = containerRef.current.getBoundingClientRect();
    const clickProgress = (e.clientX - rect.left) / rect.width;
-    onClick(Math.max(0, Math.min(1, clickProgress)));
+    onSeek(Math.max(0, Math.min(1, clickProgress)));
+  };
+
+  const handleKeyDown = (e: React.KeyboardEvent) => {
+    if (!onSeek) return;
+    const step = 0.05; // 5% step
+    if (e.key === "ArrowLeft") {
+      e.preventDefault();
+      onSeek(Math.max(0, progress - step));
+    } else if (e.key === "ArrowRight") {
+      e.preventDefault();
+      onSeek(Math.min(1, progress + step));
+    } else if (e.key === "Home") {
+      e.preventDefault();
+      onSeek(0);
+    } else if (e.key === "End") {
+      e.preventDefault();
+      onSeek(1);
+    }
  };

  return (
    <div
      ref={containerRef}
-      className="flex-1 h-2 bg-muted-foreground/20 rounded-full cursor-pointer overflow-hidden"
+      role="slider"
+      tabIndex={0}
+      aria-label="Audio progress"
+      aria-valuemin={0}
+      aria-valuemax={duration}
+      aria-valuenow={progress * duration}
+      aria-valuetext={`${formatDuration(progress * duration)} of ${formatDuration(duration)}`}
+      className="flex-1 h-2 bg-muted-foreground/20 rounded-full cursor-pointer overflow-hidden focus:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2"
      onClick={handleClick}
+      onKeyDown={handleKeyDown}
    >
      <div
        className="h-full bg-primary rounded-full transition-all"
@@ -188,6 +223,16 @@ function VoiceNotePlayer({
  const [currentTime, setCurrentTime] = useState(0);
  const [error, setError] = useState(false);

+  // Reset state when URL changes
+  useEffect(() => {
+    setError(false);
+    setIsPlaying(false);
+    setCurrentTime(0);
+    if (initialDuration) {
+      setDuration(initialDuration);
+    }
+  }, [url, initialDuration]);
+
  useEffect(() => {
    const audio = audioRef.current;
    if (!audio) return;
@@ -215,6 +260,8 @@ function VoiceNotePlayer({
    audio.addEventListener("pause", handlePause);

    return () => {
+      // Pause audio on unmount to prevent continued playback
+      audio.pause();
      audio.removeEventListener("timeupdate", handleTimeUpdate);
      audio.removeEventListener("durationchange", handleDurationChange);
      audio.removeEventListener("loadedmetadata", handleDurationChange);
@@ -223,7 +270,7 @@ function VoiceNotePlayer({
      audio.removeEventListener("play", handlePlay);
      audio.removeEventListener("pause", handlePause);
    };
-  }, []);
+  }, [url]);

  const togglePlayback = () => {
    const audio = audioRef.current;
@@ -236,11 +283,15 @@ function VoiceNotePlayer({
    }
  };

-  const handleSeek = (progress: number) => {
-    const audio = audioRef.current;
-    if (!audio || !duration) return;
-    audio.currentTime = progress * duration;
-  };
+  const handleSeek = useCallback(
+    (progress: number) => {
+      const audio = audioRef.current;
+      // Only seek if we have a valid positive duration
+      if (!audio || duration <= 0) return;
+      audio.currentTime = progress * duration;
+    },
+    [duration]
+  );

  const progress = duration > 0 ? currentTime / duration : 0;

@@ -263,7 +314,12 @@ function VoiceNotePlayer({

  return (
    <div className="flex items-center gap-3 p-3 border border-border rounded-lg bg-muted/20">
-      <audio ref={audioRef} src={url} preload="metadata" />
+      <audio
+        ref={audioRef}
+        src={url}
+        preload="metadata"
+        crossOrigin="anonymous"
+      />

      <Button
        variant="default"
@@ -283,10 +339,15 @@ function VoiceNotePlayer({
        <WaveformVisualization
          waveform={waveform}
          progress={progress}
-          onClick={handleSeek}
+          duration={duration}
+          onSeek={handleSeek}
        />
      ) : (
-        <SimpleProgressBar progress={progress} onClick={handleSeek} />
+        <SimpleProgressBar
+          progress={progress}
+          duration={duration}
+          onSeek={handleSeek}
+        />
      )}

      <div className="text-xs text-muted-foreground font-mono whitespace-nowrap">
@@ -306,12 +367,17 @@ function isVoiceNoteKind(kind: number): boolean {

 /**
 * Parent event card component for reply references
+ * Matches the API pattern from NoteRenderer
 */
 function ParentEventCard({
  parentEvent,
+  icon: Icon,
+  tooltipText,
  onClickHandler,
 }: {
  parentEvent: NostrEvent;
+  icon: LucideIcon;
+  tooltipText: string;
  onClickHandler: () => void;
 }) {
  // Don't show kind badge for common note types
@@ -326,10 +392,10 @@ function ParentEventCard({
    >
      <Tooltip>
        <TooltipTrigger asChild>
-          <Reply className="size-3 flex-shrink-0" />
+          <Icon className="size-3 flex-shrink-0" />
        </TooltipTrigger>
        <TooltipContent>
-          <p>Replying to</p>
+          <p>{tooltipText}</p>
        </TooltipContent>
      </Tooltip>
      {showKindBadge && <KindBadge kind={parentEvent.kind} variant="compact" />}
@@ -360,7 +426,7 @@ function ParentEventCard({
 */
 export function VoiceNoteRenderer({ event }: BaseEventProps) {
  const audioUrl = getAudioUrl(event);
-  const { waveform, duration } = parseVoiceNoteMetadata(event);
+  const { waveform, duration } = getVoiceNoteMetadata(event);

  if (!audioUrl) {
    return (
@@ -385,13 +451,13 @@ export function VoiceNoteRenderer({ event }: BaseEventProps) {
 }

 /**
- * Renderer for Kind 1244 - Voice Message Comment (NIP-A0)
+ * Renderer for Kind 1244 - Voice Message Reply (NIP-A0)
 * Voice message replies following NIP-22 threading
 */
 export function VoiceNoteReplyRenderer({ event }: BaseEventProps) {
  const { addWindow } = useGrimoire();
  const audioUrl = getAudioUrl(event);
-  const { waveform, duration } = parseVoiceNoteMetadata(event);
+  const { waveform, duration } = getVoiceNoteMetadata(event);

  // Use NIP-10 threading helpers (NIP-22 follows same structure)
  const refs = getNip10References(event);
@@ -429,6 +495,8 @@ export function VoiceNoteReplyRenderer({ event }: BaseEventProps) {
        {replyPointer && replyEvent && (
          <ParentEventCard
            parentEvent={replyEvent}
+            icon={Reply}
+            tooltipText="Replying to"
            onClickHandler={handleReplyClick}
          />
        )}
--- a/src/lib/imeta.ts
+++ b/src/lib/imeta.ts
@@ -13,6 +13,8 @@ export interface ImetaEntry {
  x?: string; // SHA-256 hash
  size?: string; // file size in bytes
  fallback?: string[]; // fallback URLs
+  waveform?: string; // space-separated amplitude integers (NIP-A0)
+  duration?: string; // audio duration in seconds (NIP-A0)
 }

 /**
@@ -176,3 +178,46 @@ export function getAspectRatioFromDimensions(dim?: string): string | undefined {
  // Return as CSS aspect-ratio value
  return `${width}/${height}`;
 }
+
+/**
+ * Parse waveform string into array of amplitude values
+ * Filters out any NaN values from parsing
+ */
+export function parseWaveform(waveformStr?: string): number[] | undefined {
+  if (!waveformStr) return undefined;
+
+  const values = waveformStr
+    .split(" ")
+    .map((v) => parseInt(v, 10))
+    .filter((v) => !isNaN(v));
+
+  return values.length > 0 ? values : undefined;
+}
+
+/**
+ * Parse duration string into number of seconds
+ */
+export function parseDuration(durationStr?: string): number | undefined {
+  if (!durationStr) return undefined;
+
+  const value = parseFloat(durationStr);
+  return !isNaN(value) && value >= 0 ? value : undefined;
+}
+
+/**
+ * Extract voice note metadata from imeta tags (NIP-A0)
+ * Returns waveform data and duration if present
+ */
+export function getVoiceNoteMetadata(event: NostrEvent): {
+  waveform?: number[];
+  duration?: number;
+} {
+  const imeta = parseImetaTags(event);
+  if (imeta.length === 0) return {};
+
+  const entry = imeta[0];
+  return {
+    waveform: parseWaveform(entry.waveform),
+    duration: parseDuration(entry.duration),
+  };
+}