From c03cdf5070c3c980aec3b6a6703268d0128f5a04 Mon Sep 17 00:00:00 2001 From: Orual Date: Sun, 12 Apr 2026 21:16:38 -0400 Subject: [PATCH] test: E2E tests for responsive chat/transcript layout --- assets/styling/base.css | 24 ++++ assets/styling/tabs.css | 5 + assets/styling/watch.css | 24 +++- e2e/tests/chat-responsive.spec.ts | 171 +++++++++++++++++++++++++++ e2e/tests/comments.spec.ts | 23 ++++ src/catalog.rs | 48 ++++++++ src/components/comment_connectors.rs | 105 ++++++++-------- src/components/facet_text.rs | 2 + src/components/mod.rs | 3 - src/main.rs | 10 +- src/player/transcript_panel.rs | 24 +--- 11 files changed, 352 insertions(+), 87 deletions(-) create mode 100644 e2e/tests/chat-responsive.spec.ts diff --git a/assets/styling/base.css b/assets/styling/base.css index 805a760..4833631 100644 --- a/assets/styling/base.css +++ b/assets/styling/base.css @@ -45,6 +45,30 @@ textarea { border-radius: 0; } +/* Scrollbar styling — thin, subtle, matching dark theme */ + +* { + scrollbar-width: thin; + scrollbar-color: var(--color-border) transparent; +} + +::-webkit-scrollbar { + width: 6px; + height: 6px; +} + +::-webkit-scrollbar-track { + background: transparent; +} + +::-webkit-scrollbar-thumb { + background: var(--color-border); +} + +::-webkit-scrollbar-thumb:hover { + background: var(--color-subtle); +} + /* Content fade-in after hydration / client navigation */ @keyframes fade-in { diff --git a/assets/styling/tabs.css b/assets/styling/tabs.css index 817ef9d..f0d7b47 100644 --- a/assets/styling/tabs.css +++ b/assets/styling/tabs.css @@ -24,6 +24,11 @@ color: var(--color-text); } +.tab-button:focus-visible { + outline: 1px solid var(--color-primary); + outline-offset: -1px; +} + .tab-badge { font-family: var(--font-mono); font-size: 0.7rem; diff --git a/assets/styling/watch.css b/assets/styling/watch.css index 74c20f1..12a131d 100644 --- a/assets/styling/watch.css +++ b/assets/styling/watch.css @@ -293,6 +293,14 @@ a.metadata-author { padding: 0.75rem; } + .player-wrapper { + max-width: none; + } + + .watch-tabs .chat-sidebar { + border-left: none; + } + .metadata-title { font-size: 1.1rem; } @@ -323,9 +331,10 @@ a.metadata-author { "video" "metadata" "tabs"; - grid-template-columns: minmax(0, 960px); + grid-template-columns: 1fr; grid-template-rows: auto auto 1fr; - justify-content: center; + padding: 0; + gap: 0; } .watch-sidebar-left, @@ -333,6 +342,17 @@ a.metadata-author { display: none; } + .player-wrapper { + max-width: none; + } + + .watch-metadata { + padding: 0.75rem; + } + + .watch-tabs .chat-sidebar { + border-left: none; + } } /* Narrow viewport tab container */ diff --git a/e2e/tests/chat-responsive.spec.ts b/e2e/tests/chat-responsive.spec.ts new file mode 100644 index 0000000..e9f8f58 --- /dev/null +++ b/e2e/tests/chat-responsive.spec.ts @@ -0,0 +1,171 @@ +import { test, expect } from '../fixtures/video-player-page.js'; + +// VOD with associated livestream chat from a jacquard talk. +// Used in chat-replay.spec.ts — has a livestream URI that enables the chat sidebar. +const TEST_VOD_URI = + 'at://did:plc:rbvrr34edl5ddpuwcubjiost/place.stream.video/3miacmwnd2z2z'; + +test.describe('AC4.1: Wide viewport shows sidebar layout', () => { + test.use({ viewport: { width: 1400, height: 900 } }); + + test('chat sidebar is visible and watch-tabs is hidden', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the page to fully load and chat to appear. + await expect(playerPage.page.locator('.chat-sidebar')).toBeVisible({ timeout: 30000 }); + + // The right sidebar should be visible and contain the chat sidebar. + const sidebar = playerPage.page.locator('.watch-sidebar-right'); + await expect(sidebar).toBeVisible({ timeout: 10000 }); + await expect(sidebar.locator('.chat-sidebar')).toBeVisible({ timeout: 10000 }); + + // The tabbed layout should NOT be visible on wide viewports. + const watchTabs = playerPage.page.locator('.watch-tabs'); + await expect(watchTabs).not.toBeVisible({ timeout: 5000 }); + }); + + test('transcript panel is visible in centre column', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the page to be interactive. + await expect(playerPage.videoElement).toBeVisible({ timeout: 30000 }); + + // The transcript panel should be visible in the centre column on wide viewports. + const transcriptPanel = playerPage.page.locator('.transcript-panel'); + await expect(transcriptPanel).toBeVisible({ timeout: 30000 }); + }); +}); + +test.describe('AC4.2: Narrow viewport shows tabbed layout', () => { + test.use({ viewport: { width: 768, height: 1024 } }); + + test('watch-sidebar-right is hidden and watch-tabs is visible', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the page to load. + await expect(playerPage.videoElement).toBeVisible({ timeout: 30000 }); + + // The right sidebar should NOT be visible on narrow viewports. + const sidebar = playerPage.page.locator('.watch-sidebar-right'); + await expect(sidebar).not.toBeVisible({ timeout: 10000 }); + + // The tabbed layout should be visible. + const watchTabs = playerPage.page.locator('.watch-tabs'); + await expect(watchTabs).toBeVisible({ timeout: 10000 }); + }); + + test('tab-bar contains Chat and Transcript tab buttons', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the tabbed layout to appear. + await expect(playerPage.page.locator('.watch-tabs')).toBeVisible({ timeout: 30000 }); + + // The tab bar should be present inside the tabbed container. + const tabBar = playerPage.page.locator('.watch-tabs .tab-bar'); + await expect(tabBar).toBeVisible({ timeout: 10000 }); + + // There should be tab buttons containing "Chat" and "Transcript". + const tabButtons = tabBar.locator('.tab-button'); + const buttonCount = await tabButtons.count(); + expect(buttonCount).toBeGreaterThanOrEqual(2); + + // Verify the button labels. + const buttonTexts: string[] = []; + for (let i = 0; i < buttonCount; i++) { + const text = await tabButtons.nth(i).innerText(); + buttonTexts.push(text.trim()); + } + expect(buttonTexts.some(t => t.includes('Chat'))).toBe(true); + expect(buttonTexts.some(t => t.includes('Transcript'))).toBe(true); + }); + + test('clicking Transcript tab shows transcript content', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the tabbed layout to appear. + await expect(playerPage.page.locator('.watch-tabs')).toBeVisible({ timeout: 30000 }); + + // Find and click the Transcript tab button. + const transcriptTab = playerPage.page + .locator('.watch-tabs .tab-bar .tab-button') + .filter({ hasText: 'Transcript' }); + await expect(transcriptTab).toBeVisible({ timeout: 10000 }); + await transcriptTab.click(); + + // Transcript content should become visible after clicking. + const transcriptPanel = playerPage.page.locator('.transcript-panel'); + await expect(transcriptPanel).toBeVisible({ timeout: 10000 }); + }); + + test('clicking Chat tab shows chat content', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the tabbed layout to appear. + await expect(playerPage.page.locator('.watch-tabs')).toBeVisible({ timeout: 30000 }); + + // First switch to Transcript so we have something to switch back from. + const transcriptTab = playerPage.page + .locator('.watch-tabs .tab-bar .tab-button') + .filter({ hasText: 'Transcript' }); + await expect(transcriptTab).toBeVisible({ timeout: 10000 }); + await transcriptTab.click(); + + // Now click the Chat tab. + const chatTab = playerPage.page + .locator('.watch-tabs .tab-bar .tab-button') + .filter({ hasText: 'Chat' }); + await chatTab.click(); + + // Chat content should be visible. + const chatSidebar = playerPage.page.locator('.chat-sidebar'); + await expect(chatSidebar).toBeVisible({ timeout: 10000 }); + }); + + test('AC4.4: tab state persists across navigation', async ({ playerPage }) => { + await playerPage.goto(TEST_VOD_URI); + + // Wait for the tabbed layout to appear. + const tabBar = playerPage.page.locator('.tab-bar'); + await expect(tabBar).toBeVisible({ timeout: 30000 }); + + // Click the Transcript tab. + const transcriptTab = playerPage.page.locator('.tab-button').filter({ hasText: 'Transcript' }); + await transcriptTab.click(); + await expect(transcriptTab).toHaveAttribute('data-active', 'true', { timeout: 5000 }); + + // Navigate away and back. + await playerPage.page.goto('about:blank'); + await playerPage.goto(TEST_VOD_URI); + + // Wait for tabs to reappear. + await expect(playerPage.page.locator('.tab-bar')).toBeVisible({ timeout: 30000 }); + + // Transcript tab should still be active (SessionStorage persistence). + const transcriptTabAfter = playerPage.page + .locator('.tab-button') + .filter({ hasText: 'Transcript' }); + await expect(transcriptTabAfter).toHaveAttribute('data-active', 'true', { timeout: 10000 }); + }); +}); + +test.describe('AC4.5: comment badge on narrow viewport', () => { + test.use({ viewport: { width: 768, height: 1024 } }); + + test('transcript tab shows comment count badge', async ({ playerPage }) => { + // This VOD has ionosphere comments — the Transcript tab badge should show a count. + const COMMENT_VOD = + 'at://did:plc:rbvrr34edl5ddpuwcubjiost/place.stream.video/3mi5a2y3tej2z'; + await playerPage.goto(COMMENT_VOD); + + // Wait for the tabbed layout to appear. + const tabBar = playerPage.page.locator('.tab-bar'); + await expect(tabBar).toBeVisible({ timeout: 30000 }); + + // The Transcript tab should have a badge showing the comment count. + const badge = playerPage.page.locator('.tab-badge'); + await expect(badge).toBeVisible({ timeout: 30000 }); + + const badgeText = await badge.innerText(); + expect(parseInt(badgeText, 10)).toBeGreaterThan(0); + }); +}); diff --git a/e2e/tests/comments.spec.ts b/e2e/tests/comments.spec.ts index ea3e1f1..fa88579 100644 --- a/e2e/tests/comments.spec.ts +++ b/e2e/tests/comments.spec.ts @@ -160,4 +160,27 @@ test.describe('Comment Rendering on Transcript', () => { await expect(expandedBodies.first()).toBeVisible({ timeout: 5000 }); }); }); + + test('AC2.10: VOD without transcript shows no comment infrastructure', async ({ + playerPage, + }) => { + // This VOD has a livestream (chat works) but no ionosphere transcript/talk, + // so no comment anchors or comment headers should be present. + const NO_TRANSCRIPT_VOD = + 'at://did:plc:rbvrr34edl5ddpuwcubjiost/place.stream.video/3miacmwnd2z2z'; + await playerPage.goto(NO_TRANSCRIPT_VOD); + + // Wait for page to load — chat messages should appear in the sidebar. + await expect(playerPage.page.locator('.chat-message').first()).toBeVisible({ + timeout: 30000, + }); + + // No comment anchors should be present — this VOD has no transcript. + const anchors = playerPage.page.locator('.comment-anchor'); + await expect(anchors).toHaveCount(0, { timeout: 5000 }); + + // No non-anchored comment header either. + const commentHeader = playerPage.page.locator('.transcript-comments-header'); + await expect(commentHeader).not.toBeAttached({ timeout: 5000 }); + }); }); diff --git a/src/catalog.rs b/src/catalog.rs index d619e57..f063699 100644 --- a/src/catalog.rs +++ b/src/catalog.rs @@ -1123,6 +1123,21 @@ pub async fn get_comments( let constellation_uri = Uri::parse(CONSTELLATION_URL) .map_err(|e| ServerFnError::new(format!("bad constellation URL: {e}")))?; + // Discover transcript URI server-side if not provided, so we also find + // comments whose subject is the transcript (not just the talk). + let transcript_uri = match transcript_uri { + Some(uri) => Some(uri), + None => { + query_backlinks(&client, talk_uri.as_str(), "tv.ionosphere.transcript:talkUri") + .await + .and_then(|bl| { + bl.records.first().and_then(|rec| { + AtUri::from_parts_owned(&rec.did, rec.collection.as_ref(), &rec.rkey).ok() + }) + }) + } + }; + // Collect all unique comment AT URIs from backlink queries. // We query for comments on the talk and, if provided, the transcript. let mut comment_uri_strs: Vec = Vec::new(); @@ -2401,4 +2416,37 @@ mod tests { assert!(video.talk.is_none()); assert!(video.speakers.is_empty()); } + + #[test] + fn comment_data_serialization_roundtrip() { + use crate::chat::{FacetData, FacetFeatureData}; + + let original = CommentData { + uri: "at://did:plc:test000000000000000000000001/tv.ionosphere.comment/abc" + .parse() + .unwrap(), + text: SmolStr::new("test comment"), + author_did: "did:plc:test000000000000000000001".parse().unwrap(), + author_handle: "test.bsky.social".parse().unwrap(), + created_at: "2026-04-01T12:00:00.000Z".parse().unwrap(), + anchor: Some(ByteRangeData { + byte_start: 10, + byte_end: 20, + }), + subject_uri: "at://did:plc:test000000000000000000000001/tv.ionosphere.talk/xyz" + .parse() + .unwrap(), + facets: Some(vec![FacetData { + byte_start: 0, + byte_end: 4, + features: vec![FacetFeatureData::Link { + uri: SmolStr::new("https://example.com"), + }], + }]), + }; + + let json = serde_json::to_string(&original).expect("serialize"); + let roundtripped: CommentData = serde_json::from_str(&json).expect("deserialize"); + assert_eq!(original, roundtripped); + } } diff --git a/src/components/comment_connectors.rs b/src/components/comment_connectors.rs index c2779e2..cc9cdb3 100644 --- a/src/components/comment_connectors.rs +++ b/src/components/comment_connectors.rs @@ -33,6 +33,9 @@ pub fn CommentConnectors( let mut svg_height = use_signal(|| 0.0_f64); // Shared recalculation function: measure DOM and update paths. + // The spawned task may outlive the component (e.g. tab switch unmounts + // CommentConnectors while rAF is pending), so signal writes use + // try_write to avoid panicking on dropped signals. let recalculate = move || { wasm_bindgen_futures::spawn_local(async move { // Yield to the browser's next animation frame for settled layout. @@ -44,8 +47,10 @@ pub fn CommentConnectors( let _ = wasm_bindgen_futures::JsFuture::from(promise).await; let (height, new_paths) = compute_connector_paths(); - svg_height.set(height); - paths.set(new_paths); + // Guard: signals may have been dropped if the component + // unmounted while we were awaiting the animation frame. + let _ = svg_height.try_write().map(|mut v| *v = height); + let _ = paths.try_write().map(|mut v| *v = new_paths); }); }; @@ -57,14 +62,12 @@ pub fn CommentConnectors( recalculate(); }); - // Set up ResizeObserver once on mount. Guard prevents re-creation - // if the effect re-runs (e.g. on container remount). - let mut observer_installed = use_signal(|| false); + // Set up ResizeObserver — stored in a signal so it's disconnected on unmount. + let mut observer_handle: Signal> = use_signal(|| None); use_effect(move || { let _container = container_ref.read(); - if !*observer_installed.peek() { - setup_resize_observer(recalculate); - observer_installed.set(true); + if observer_handle.peek().is_none() { + observer_handle.set(setup_resize_observer(recalculate)); } }); @@ -261,67 +264,53 @@ fn find_parent_comment_body(body_el: &web_sys::Element) -> Option, +} + +#[cfg(all(target_family = "wasm", target_os = "unknown"))] +impl Drop for ObserverHandle { + fn drop(&mut self) { + self.observer.disconnect(); + } +} + /// Set up a `ResizeObserver` on the `.transcript-panel` element that invokes -/// `on_resize` whenever the panel's dimensions change, debounced to ~100ms. +/// `on_resize` whenever the panel's dimensions change, debounced to 100ms. /// -/// The observer and its closure are intentionally leaked — they live for the -/// lifetime of the transcript panel. Since there is at most one panel (and thus -/// one observer), this is acceptable. +/// Returns an `ObserverHandle` — dropping it disconnects the observer and frees +/// the closure. Store the handle in a signal so it's cleaned up on unmount. #[cfg(all(target_family = "wasm", target_os = "unknown"))] -fn setup_resize_observer(on_resize: impl Fn() + 'static) { - use std::cell::Cell; +fn setup_resize_observer(on_resize: impl Fn() + 'static) -> Option { + use std::cell::RefCell; use std::rc::Rc; use wasm_bindgen::prelude::*; - let doc = match web_sys::window().and_then(|w| w.document()) { - Some(d) => d, - None => return, - }; + let doc = web_sys::window().and_then(|w| w.document())?; + let container = doc.query_selector(".transcript-panel").ok().flatten()?; - let container = match doc.query_selector(".transcript-panel").ok().flatten() { - Some(el) => el, - None => return, - }; - - // Debounce: only fire after 100ms of no resize events. - let timer_id: Rc> = Rc::new(Cell::new(0)); + // Debounce via gloo_timers — dropping the previous Timeout cancels it. let on_resize = Rc::new(on_resize); + let pending: Rc>> = + Rc::new(RefCell::new(None)); - let timer_id_clone = timer_id.clone(); - let on_resize_clone = on_resize.clone(); - - let callback = Closure::::new( + let callback = Closure::::new({ + let pending = pending.clone(); + let on_resize = on_resize.clone(); move |_entries: js_sys::Array, _observer: web_sys::ResizeObserver| { - if let Some(win) = web_sys::window() { - // Clear any pending debounce timer. - let prev = timer_id_clone.get(); - if prev != 0 { - win.clear_timeout_with_handle(prev); - } - // Set a new 100ms debounce timer. - let on_resize_inner = on_resize_clone.clone(); - let debounce_cb = Closure::::new(move || { - on_resize_inner(); - }); - if let Ok(id) = win.set_timeout_with_callback_and_timeout_and_arguments_0( - debounce_cb.as_ref().unchecked_ref(), - 100, - ) { - timer_id_clone.set(id); - } - debounce_cb.forget(); - } - }, - ); + let on_resize = on_resize.clone(); + *pending.borrow_mut() = Some(gloo_timers::callback::Timeout::new(100, move || { + on_resize(); + })); + } + }); - let Ok(observer) = - web_sys::ResizeObserver::new(callback.as_ref().unchecked_ref()) - else { - return; - }; + let observer = web_sys::ResizeObserver::new(callback.as_ref().unchecked_ref()).ok()?; observer.observe(&container); - // Leak both so they live for the page lifetime. - callback.forget(); - std::mem::forget(observer); + Some(ObserverHandle { observer, _callback: callback }) } diff --git a/src/components/facet_text.rs b/src/components/facet_text.rs index f7ee382..79ebc52 100644 --- a/src/components/facet_text.rs +++ b/src/components/facet_text.rs @@ -162,6 +162,7 @@ pub fn FacetText(text: SmolStr, facets: Vec) -> Element { a { href: "https://bsky.app/profile/{did}", target: "_blank", + rel: "noopener noreferrer", {t.as_str()} } }, @@ -177,6 +178,7 @@ pub fn FacetText(text: SmolStr, facets: Vec) -> Element { a { href: "https://bsky.app/search?q=%23{tag}", target: "_blank", + rel: "noopener noreferrer", {t.as_str()} } }, diff --git a/src/components/mod.rs b/src/components/mod.rs index 0d74b24..5dde84b 100644 --- a/src/components/mod.rs +++ b/src/components/mod.rs @@ -5,7 +5,4 @@ pub mod facet_text; pub mod tab_bar; pub mod video_card; pub use at_uri_display::AtUriDisplay; -pub use comment_body::CommentBody; -pub use facet_text::FacetText; -pub use tab_bar::{TabBar, TabDef}; pub use video_card::VideoCard; diff --git a/src/main.rs b/src/main.rs index c57f488..1a4b34d 100644 --- a/src/main.rs +++ b/src/main.rs @@ -163,6 +163,9 @@ fn Watch(uri: SmolStr) -> Element { let mut is_narrow: Signal = use_signal(|| false); #[cfg(all(target_family = "wasm", target_os = "unknown"))] { + // Store the listener in a signal so it's dropped when the component unmounts, + // preventing accumulation of leaked listeners across navigations. + let mut _mql_listener: Signal> = use_signal(|| None); use_effect(move || { use wasm_bindgen::JsCast; @@ -175,21 +178,18 @@ fn Watch(uri: SmolStr) -> Element { _ => return, }; - // Set initial value. is_narrow.set(mql.matches()); - // Listen for changes via gloo_events (FnMut-compatible, manages lifetime). let listener = gloo_events::EventListener::new( mql.unchecked_ref::(), "change", move |event| { if let Ok(mql_event) = event.clone().dyn_into::() { - is_narrow.set(mql_event.matches()); + let _ = is_narrow.try_write().map(|mut v| *v = mql_event.matches()); } }, ); - // Leak the listener — it must live for the lifetime of the page. - listener.forget(); + _mql_listener.set(Some(listener)); // Dropped when Watch unmounts. }); } diff --git a/src/player/transcript_panel.rs b/src/player/transcript_panel.rs index e6453a0..5904f8c 100644 --- a/src/player/transcript_panel.rs +++ b/src/player/transcript_panel.rs @@ -59,24 +59,10 @@ fn find_active_range(words: &[TranscriptWord], current_ms: u64) -> Option usize { - 1 + node.replies.iter().map(count_comment_node).sum::() -} - -/// Count all comments in a `CommentTree` (anchored + non-anchored + all replies). -fn count_all_comments(tree: &CommentTree) -> usize { - let anchored_count: usize = tree - .anchored - .iter() - .map(|a| count_comment_node(&a.comment_node)) - .sum(); - let non_anchored_count: usize = tree - .non_anchored - .iter() - .map(count_comment_node) - .sum(); - anchored_count + non_anchored_count +/// Count top-level comments in a `CommentTree` (anchored + non-anchored, excluding replies). +/// Used consistently for both the toggle button and tab badge. +fn count_top_level_comments(tree: &CommentTree) -> usize { + tree.anchored.len() + tree.non_anchored.len() } /// Scrolling transcript panel with word-level highlighting and click-to-seek. @@ -235,7 +221,7 @@ pub fn TranscriptPanel( .collect(); // Total comment count (anchored + non-anchored + all replies recursively). - let comment_count = tree_ref.map(count_all_comments).unwrap_or(0); + let comment_count = tree_ref.map(count_top_level_comments).unwrap_or(0); // Panel class includes the hidden modifier when the master toggle is off. let panel_class = if *comments_visible.read() { -- 2.51.2