diff --git a/crates/op-editor-ui/src/widgets/mod.rs b/crates/op-editor-ui/src/widgets/mod.rs index e208074bb..a1875106b 100644 --- a/crates/op-editor-ui/src/widgets/mod.rs +++ b/crates/op-editor-ui/src/widgets/mod.rs @@ -133,6 +133,7 @@ pub mod property_panel_widget; #[cfg(test)] mod property_panel_widget_tests; mod scene_template_card_actions; +mod scene_template_caret; mod scene_template_panel_paint; pub(crate) mod scene_template_previews; mod scene_template_style_paint; diff --git a/crates/op-editor-ui/src/widgets/scene_template_caret.rs b/crates/op-editor-ui/src/widgets/scene_template_caret.rs new file mode 100644 index 000000000..e9a02ac5a --- /dev/null +++ b/crates/op-editor-ui/src/widgets/scene_template_caret.rs @@ -0,0 +1,123 @@ +//! Where the caret is in the Asset Center's focused field. +//! +//! Only one consumer, and it is not paint: the hosts hand this rect to the +//! platform so an IME candidate window opens under the text being composed +//! rather than at the pointer or at the window origin. Paint derives its own +//! caret from the same `TextInputState`, so the two cannot disagree about the +//! character position — only about which field is live, and that question is +//! answered here by the same `field_focused` predicate paint uses. + +use op_editor_core::SceneTemplateFocus; + +use super::scene_template_panel::{ + SceneTemplatePanel, GENERATE_INPUT_PAD_X, GENERATE_TEXT_SIZE, SEARCH_PAD_X, SEARCH_TEXT_SIZE, +}; +use super::text_input::single_line_caret_rect; +use crate::Rect; + +impl SceneTemplatePanel<'_> { + /// Caret rect of whichever field the keyboard is writing into. + /// + /// Falls back to the search field rather than returning `None`: the panel + /// always has a focused field while it is open, and a `None` here would + /// send the candidate window back to the pointer-position fallback for a + /// field that is right there on screen. + pub fn focused_input_caret_rect(&self, panel: Rect) -> Rect { + let center = &self.state.editor_ui.scene_template_center; + // `field_focused` already resolves the one case where the stored + // focus and the painted panel disagree — a topic field whose row the + // scene filter has hidden hands focus back to search. + if self.field_focused(SceneTemplateFocus::Generate) { + if let Some(input) = self.generate_input_rect(panel) { + return single_line_caret_rect( + ¢er.generate, + input, + GENERATE_TEXT_SIZE, + GENERATE_INPUT_PAD_X, + ); + } + } + single_line_caret_rect( + ¢er.search, + Self::search_rect(panel), + SEARCH_TEXT_SIZE, + SEARCH_PAD_X, + ) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::widgets::scene_template_panel::test_rects::MEDIUM as PANEL; + use op_editor_core::EditorState; + + fn open_state() -> EditorState { + let mut state = EditorState::default(); + state.editor_ui.scene_template_generate_supported = true; + state.editor_ui.open_scene_template_center(0); + state + } + + /// The caret tracks the focused field, not a fixed one — anchoring the + /// candidate window over the search box while the user types a topic is + /// the bug this rect exists to prevent. + #[test] + fn the_caret_follows_focus_between_the_two_fields() { + let mut state = open_state(); + let panel = SceneTemplatePanel::for_editor(&state).expect("open"); + let search = panel.focused_input_caret_rect(PANEL); + assert!( + SceneTemplatePanel::search_rect(PANEL).contains(search.origin), + "search focus must anchor in the search field" + ); + + state.editor_ui.scene_template_center.focus = SceneTemplateFocus::Generate; + let panel = SceneTemplatePanel::for_editor(&state).expect("open"); + let topic = panel.focused_input_caret_rect(PANEL); + let input = panel.generate_input_rect(PANEL).expect("the row paints"); + assert!( + input.contains(topic.origin), + "topic focus must anchor in the topic field" + ); + // Both fields start at the same x (same content column, same glyph + // inset), so the row is what separates them. + assert!(topic.origin.y > search.origin.y); + } + + /// A focus on a row the scene filter has hidden falls back to search, + /// matching what paint does with the same predicate. + #[test] + fn a_hidden_topic_row_anchors_at_the_search_field() { + let mut state = open_state(); + state.editor_ui.scene_template_center.focus = SceneTemplateFocus::Generate; + state.editor_ui.scene_template_center.filter = op_editor_core::SceneFilter::Scene( + op_editor_core::scene_template_catalog::TemplateScene::Card, + ); + + let panel = SceneTemplatePanel::for_editor(&state).expect("open"); + assert!(panel.generate_input_rect(PANEL).is_none()); + let caret = panel.focused_input_caret_rect(PANEL); + assert!(SceneTemplatePanel::search_rect(PANEL).contains(caret.origin)); + } + + /// The caret advances as text is typed, so the candidate window follows + /// the composition instead of sitting at the field's left edge. + #[test] + fn the_caret_advances_with_the_text() { + let mut state = open_state(); + let empty = SceneTemplatePanel::for_editor(&state) + .expect("open") + .focused_input_caret_rect(PANEL); + + state + .editor_ui + .scene_template_center + .search + .set_text("presentation"); + let typed = SceneTemplatePanel::for_editor(&state) + .expect("open") + .focused_input_caret_rect(PANEL); + assert!(typed.origin.x > empty.origin.x); + } +} diff --git a/crates/op-host-native/src/widget_host/ime.rs b/crates/op-host-native/src/widget_host/ime.rs index 5e371d260..8ee932256 100644 --- a/crates/op-host-native/src/widget_host/ime.rs +++ b/crates/op-host-native/src/widget_host/ime.rs @@ -50,6 +50,12 @@ impl WidgetHostNative { } return had; } + if self.editor_state.editor_ui.scene_template_center.open { + if had { + self.mark_dirty(); + } + return had; + } if self.editor_state.editor_ui.image_panel.search_open || self.editor_state.editor_ui.image_panel.generate_open { @@ -101,6 +107,19 @@ impl WidgetHostNative { } return consumed; } + // Above the canvas-text branch on purpose: the gallery covers the + // canvas, so a text node left mid-edit underneath it must not take + // the candidate the user composed into the panel. Same stale-focus + // rule the Prompt Center branch above encodes. + if self.editor_state.editor_ui.scene_template_center.open { + let mut consumed = false; + for ch in text.chars() { + if !ch.is_control() && self.apply_text(ch) { + consumed = true; + } + } + return consumed; + } if self.editor_state.editor_ui.image_panel.search_open || self.editor_state.editor_ui.image_panel.generate_open { @@ -159,6 +178,12 @@ impl WidgetHostNative { ) { return Some(panel.focused_input_caret_rect(rect)); } + if let (Some(panel), Some(rect)) = ( + op_editor_ui::widgets::SceneTemplatePanel::for_editor(&self.editor_state), + self.scene_template_panel_rect(viewport_w, viewport_h), + ) { + return Some(panel.focused_input_caret_rect(rect)); + } if image_popover_open { self.editor_state .editor_ui diff --git a/crates/op-host-native/src/widget_host/input.rs b/crates/op-host-native/src/widget_host/input.rs index a3ab9565e..470818d88 100644 --- a/crates/op-host-native/src/widget_host/input.rs +++ b/crates/op-host-native/src/widget_host/input.rs @@ -55,6 +55,15 @@ impl WidgetHostNative { && self.editor_state.editor_ui.agent_settings.focus.is_some()) || self.editor_state.editor_ui.icon_picker.open || self.editor_state.editor_ui.prompt_center.open + // The Asset Center owns the keyboard the whole time it is open — + // it always has one of its two fields focused, and + // `scene_template_keyboard` swallows every key regardless. Its + // absence here is what left the gallery unable to take IME input: + // `text_input_focus_active` reads this list, and a `false` there + // makes the desktop shell call `set_ime_allowed(false)`, so the + // platform never opens a composition session and pinyin produced + // nothing at all while ASCII still went through `apply_text`. + || self.editor_state.editor_ui.scene_template_center.open || self.editor_state.editor_ui.chat_model_picker.open || self.editor_state.editor_ui.component_browser_open || self.editor_state.chat.focused diff --git a/crates/op-host-native/src/widget_host/scene_template_ime_tests.rs b/crates/op-host-native/src/widget_host/scene_template_ime_tests.rs new file mode 100644 index 000000000..1b8f587b9 --- /dev/null +++ b/crates/op-host-native/src/widget_host/scene_template_ime_tests.rs @@ -0,0 +1,192 @@ +//! Contract tests for typing into the Asset Center. +//! +//! The gallery is a full-canvas overlay with two text fields, and it reached +//! users unable to accept IME input: `input_active` — the one list of surfaces +//! that own the keyboard — did not name it, so `text_input_focus_active` read +//! false, the desktop shell called `set_ime_allowed(false)`, and the platform +//! never opened a composition session. Most Latin characters still worked, +//! because they arrive as ordinary key events and `apply_text` has always +//! routed the panel — but the same list gates the single-letter tool +//! switches, so `v r o l t f p y h` were consumed by the toolbar instead and +//! silently changed the canvas tool behind the overlay. That asymmetry is +//! what made the bug read as "the caret blinks but nothing types" to anyone +//! using pinyin, and as nothing at all to anyone testing with `abc`. +//! +//! So each field is pinned on both roads: a plain character and a committed +//! candidate. A test that only drove `apply_text` would have stayed green +//! through the entire outage. + +use crate::WidgetHostNative; +use op_editor_core::{EditorState, NodeId, SceneTemplateFocus}; + +const TEXT_DOC: &str = r#"{"version":"1.0.0","children":[ + {"type":"text","id":"t1","name":"Label","x":0,"y":0,"width":100,"height":40, + "content":"","fontSize":20} +]}"#; + +/// The panel open on the tab a user lands on, with generation available. +fn gallery_host() -> WidgetHostNative { + let mut host = WidgetHostNative::new(); + host.editor_state_mut() + .editor_ui + .scene_template_generate_supported = true; + host.editor_state_mut() + .editor_ui + .open_scene_template_center(0); + host +} + +/// The state "generate from this" leaves behind: topic field focused without +/// the user ever having clicked it. +fn topic_focused_host() -> WidgetHostNative { + let mut host = gallery_host(); + let template = op_editor_core::scene_template_catalog::scene_template_by_id("slide-deck") + .expect("the deck template ships"); + assert!(host + .editor_state_mut() + .editor_ui + .use_scene_template_as_generate_basis(template)); + assert_eq!( + host.editor_state().editor_ui.scene_template_center.focus, + SceneTemplateFocus::Generate + ); + host +} + +fn topic(host: &WidgetHostNative) -> String { + host.editor_state() + .editor_ui + .scene_template_center + .generate + .text() + .to_string() +} + +fn search(host: &WidgetHostNative) -> String { + host.editor_state() + .editor_ui + .scene_template_center + .search + .text() + .to_string() +} + +/// The gate the whole bug hung on. Without this the shell disables IME and +/// no amount of correct routing below ever sees an event. +#[test] +fn an_open_gallery_owns_the_keyboard_for_ime_purposes() { + let plain = WidgetHostNative::new(); + assert!(!plain.text_input_focus_active()); + + assert!( + gallery_host().text_input_focus_active(), + "the Asset Center must report a live text input, or the desktop \ + shell turns the platform IME off while the caret is blinking" + ); +} + +/// The same list gates the desktop shell's single-letter tool switches +/// (`keyboard_input.rs`: `Key::Character(..) if !input_active_pub()`), which +/// consume the key when they fire. While the gallery was missing from it, a +/// `t` typed into the search box switched the canvas tool to Text and never +/// reached the field — nine letters of the alphabet were unusable and each +/// one quietly changed the document's tool behind the overlay. +#[test] +fn an_open_gallery_suppresses_the_single_letter_tool_shortcuts() { + assert!( + gallery_host().input_active_pub(), + "tool shortcuts would eat letters typed into the gallery" + ); +} + +/// Programmatic focus, then a pinyin candidate. This is the user's exact +/// sequence: press "generate from this", type a Chinese topic. +#[test] +fn a_committed_candidate_lands_in_the_programmatically_focused_topic_field() { + let mut host = topic_focused_host(); + + assert!(host.apply_ime_commit("季度复盘")); + + assert_eq!(topic(&host), "季度复盘"); + assert!( + search(&host).is_empty(), + "the topic must not leak into search" + ); +} + +/// The Latin road through the same focus — green before the fix, and the +/// reason the outage read as "only Chinese is broken". +#[test] +fn plain_characters_land_in_the_programmatically_focused_topic_field() { + let mut host = topic_focused_host(); + + for c in "Q3".chars() { + assert!(host.apply_text(c)); + } + + assert_eq!(topic(&host), "Q3"); +} + +/// The other field, reached by its own focus rather than by the card button. +#[test] +fn the_search_field_takes_both_roads_too() { + let mut host = gallery_host(); + assert_eq!( + host.editor_state().editor_ui.scene_template_center.focus, + SceneTemplateFocus::Search + ); + + assert!(host.apply_text('P')); + assert!(host.apply_ime_commit("演示")); + + assert_eq!(search(&host), "P演示"); + assert!(topic(&host).is_empty()); +} + +/// A canvas text node left mid-edit sits underneath the gallery. The +/// candidate belongs to the panel the user is looking at, not to the node +/// the overlay is covering. +#[test] +fn the_gallery_beats_a_stale_canvas_text_edit_and_chat_focus() { + let mut host = WidgetHostNative::new(); + let doc = jian_ops_schema::load_str(TEXT_DOC) + .expect("fixture JSON parses") + .value; + *host.editor_state_mut() = EditorState::from_document(doc); + assert!(host.editor_state_mut().start_text_edit(NodeId::new("t1"))); + host.editor_state_mut().chat.focused = true; + host.editor_state_mut() + .editor_ui + .scene_template_generate_supported = true; + host.editor_state_mut() + .editor_ui + .open_scene_template_center(0); + + assert!(host.apply_ime_commit("模板")); + + assert_eq!(search(&host), "模板"); + assert!( + host.editor_state().chat.input.text().is_empty(), + "stale chat focus must not take the commit" + ); +} + +/// The candidate window has to open under the text being composed. Anchoring +/// it at the pointer — the fallback when no rect resolves — puts the +/// candidate list somewhere unrelated to the field. +#[test] +fn the_candidate_window_anchors_at_the_focused_field() { + let mut host = topic_focused_host(); + + let rect = host + .ime_anchor_rect(1440.0, 900.0) + .expect("an open gallery resolves an anchor"); + + let panel_rect = host + .scene_template_panel_rect(1440.0, 900.0) + .expect("the panel has a rect while open"); + assert!( + panel_rect.contains(rect.origin), + "the anchor landed outside the gallery: {rect:?}" + ); +} diff --git a/crates/op-host-web/src/widget_host/keyboard.rs b/crates/op-host-web/src/widget_host/keyboard.rs index 373bee5fb..ad2edef15 100644 --- a/crates/op-host-web/src/widget_host/keyboard.rs +++ b/crates/op-host-web/src/widget_host/keyboard.rs @@ -21,6 +21,16 @@ impl WidgetHost { } return true; } + // Native parity (`op-host-native/widget_host/keyboard.rs`). Without + // this the web Asset Center could not be typed into at all — a + // character fell through to the canvas shortcuts, where a bare letter + // switches tools behind the open gallery. + if let Some(changed) = shared::scene_template_text(&mut self.editor_state, c, self.now_ms) { + if changed { + self.mark_dirty(); + } + return true; + } if self.editor_state.editor_ui.collab_join_input_active() { let changed = op_editor_ui::widgets::collab_ui::join_address_text( &mut self.editor_state.editor_ui, diff --git a/crates/op-host-web/src/widget_host/keyboard_ime.rs b/crates/op-host-web/src/widget_host/keyboard_ime.rs index 276ae4f8c..1da04eb39 100644 --- a/crates/op-host-web/src/widget_host/keyboard_ime.rs +++ b/crates/op-host-web/src/widget_host/keyboard_ime.rs @@ -38,6 +38,12 @@ impl WidgetHost { ) { return Some(panel.focused_input_caret_rect(rect)); } + if let (Some(panel), Some(rect)) = ( + op_editor_ui::widgets::SceneTemplatePanel::for_editor(&self.editor_state), + self.scene_template_panel_rect(self.last_viewport_w, self.last_viewport_h), + ) { + return Some(panel.focused_input_caret_rect(rect)); + } if image_popover_open { self.editor_state .editor_ui diff --git a/crates/op-host-web/src/widget_host/keyboard_text_inputs.rs b/crates/op-host-web/src/widget_host/keyboard_text_inputs.rs index d3ab78c37..cdc15f97a 100644 --- a/crates/op-host-web/src/widget_host/keyboard_text_inputs.rs +++ b/crates/op-host-web/src/widget_host/keyboard_text_inputs.rs @@ -36,6 +36,11 @@ impl WidgetHost { && self.editor_state.editor_ui.agent_settings.focus.is_some()) || self.editor_state.editor_ui.icon_picker.open || self.editor_state.editor_ui.prompt_center.open + // The Asset Center always has one of its two fields focused and + // swallows every key. Native carries the same entry; leaving it + // out here kept the hidden IME input from ever taking DOM focus, + // so composition events never reached the panel. + || self.editor_state.editor_ui.scene_template_center.open || self.editor_state.editor_ui.chat_model_picker.open || self.editor_state.editor_ui.component_browser_open || self.editor_state.chat.focused @@ -60,6 +65,7 @@ impl WidgetHost { || editor_ui.preset_name_input_active() || editor_ui.icon_picker.open || editor_ui.prompt_center.open + || editor_ui.scene_template_center.open || editor_ui.component_browser_open || self.git_commit_focus_active() || self.git_remote_focus_active() @@ -159,6 +165,12 @@ impl WidgetHost { }; return slice(input); } + if eui.scene_template_center.open { + // Copying from the gallery's own fields, not from whatever the + // canvas had selected behind it. + return op_editor_core::scene_template_keyboard::selected_text(&self.editor_state) + .map(str::to_string); + } if eui.agent_settings.focus.is_some() { return slice(&eui.settings_input); } diff --git a/crates/op-host-web/src/widget_host/scene_template_ime_tests.rs b/crates/op-host-web/src/widget_host/scene_template_ime_tests.rs new file mode 100644 index 000000000..bcb6dba7a --- /dev/null +++ b/crates/op-host-web/src/widget_host/scene_template_ime_tests.rs @@ -0,0 +1,110 @@ +//! Contract tests for typing into the Asset Center on the web host. +//! +//! Web was worse off than native: besides missing from `input_active` — which +//! is what gives the hidden capture input DOM focus, and therefore the only +//! reason composition events ever arrive — `apply_text` had no branch for the +//! panel at all. A character typed into the gallery fell through to the +//! canvas shortcuts, where a bare letter switches the active tool behind the +//! open overlay. + +use super::WidgetHost; +use op_editor_core::SceneTemplateFocus; + +fn gallery_host() -> WidgetHost { + let mut host = WidgetHost::new(); + host.last_viewport_w = 1440.0; + host.last_viewport_h = 900.0; + host.editor_state.editor_ui.open_scene_template_center(0); + host +} + +fn search(host: &WidgetHost) -> String { + host.editor_state + .editor_ui + .scene_template_center + .search + .text() + .to_string() +} + +/// The gate the browser's hidden IME input hangs off. +#[test] +fn an_open_gallery_owns_the_keyboard_for_ime_purposes() { + assert!(!WidgetHost::new().text_input_focus_active()); + assert!(gallery_host().text_input_focus_active()); +} + +/// `apply_text` had no branch for the panel at all, so every character fell +/// through to the canvas paths behind the overlay. +#[test] +fn plain_characters_reach_the_panel() { + let mut host = gallery_host(); + + for c in "rect".chars() { + assert!(host.apply_text(c)); + } + + assert_eq!(search(&host), "rect"); +} + +/// The gate the tool shortcuts read, so a letter typed into the gallery is +/// never also a tool switch. +#[test] +fn an_open_gallery_suppresses_the_editor_shortcuts() { + assert!(gallery_host().input_active()); +} + +/// A composed candidate arrives through the same door a paste does. +#[test] +fn a_committed_candidate_lands_in_the_focused_field() { + let mut host = gallery_host(); + host.editor_state.editor_ui.scene_template_center.focus = SceneTemplateFocus::Generate; + + assert!(host.apply_paste_text("季度复盘")); + + assert_eq!( + host.editor_state + .editor_ui + .scene_template_center + .generate + .text(), + "季度复盘" + ); + assert!(search(&host).is_empty()); +} + +/// The candidate window anchors at the field, not at the last pointer +/// position — the fallback this branch exists to replace. +#[test] +fn the_candidate_window_anchors_at_the_focused_field() { + let mut host = gallery_host(); + host.last_cursor_x = 20.0; + host.last_cursor_y = 20.0; + + let rect = host.ime_anchor_rect().expect("an open gallery anchors"); + let panel_rect = host + .scene_template_panel_rect(1440.0, 900.0) + .expect("the panel has a rect while open"); + + assert!( + panel_rect.contains(rect.origin), + "the anchor fell back to the pointer: {rect:?}" + ); +} + +/// Copy takes the gallery's own selection rather than whatever the canvas +/// had selected behind the overlay. +#[test] +fn copy_reads_the_gallery_field() { + let mut host = gallery_host(); + { + let input = &mut host.editor_state.editor_ui.scene_template_center.search; + input.set_text("演示文稿"); + input.select_all(); + } + + assert_eq!( + host.focused_input_selected_text().as_deref(), + Some("演示文稿") + ); +}