From a357b22778e82e8dc75d853ecd70f04ed32a1ca7 Mon Sep 17 00:00:00 2001 From: "sorokin.oleg.edu" Date: Sat, 12 Sep 2026 17:39:40 -0700 Subject: [PATCH] Make custom overlay keyboard targets and controls distinct --- crates/oa-desktop/src/context_menu.rs | 150 +++++++++++++- .../src/forms/reference_chooser/trigger.rs | 2 +- .../src/forms/reference_chooser/widget.rs | 186 +++++++++++++++++- 3 files changed, 335 insertions(+), 3 deletions(-) diff --git a/crates/oa-desktop/src/context_menu.rs b/crates/oa-desktop/src/context_menu.rs index 34a3ef6f..2d701a16 100644 --- a/crates/oa-desktop/src/context_menu.rs +++ b/crates/oa-desktop/src/context_menu.rs @@ -737,6 +737,25 @@ impl overlay::Overlay for MenuOverlay< }, Color::from_rgb8(218, 238, 235), ); + if row.width > 2. && row.height > 2. { + renderer.fill_quad( + renderer::Quad { + bounds: Rectangle { + x: row.x + 1., + y: row.y + 1., + width: row.width - 2., + height: row.height - 2., + }, + border: Border { + color: style::TEAL, + width: 1.5, + radius: 4.into(), + }, + ..Default::default() + }, + Color::TRANSPARENT, + ); + } } let color = if message.is_some() { style::INK @@ -792,7 +811,7 @@ impl overlay::Overlay for MenuOverlay< }, ..Default::default() }, - style::LINE, + style::BOUNDARY, ); } }); @@ -1103,6 +1122,135 @@ impl<'a, Message: 'a> From> for Element<'a, Message> { mod tests { use super::*; + fn pixel_contrast_on_white(pixel: &[u8]) -> f32 { + let linear = |byte: u8| { + let value = f32::from(byte) / 255.; + if value <= 0.04045 { + value / 12.92 + } else { + ((value + 0.055) / 1.055).powf(2.4) + } + }; + 1.05 / (0.2126 * linear(pixel[0]) + + 0.7152 * linear(pixel[1]) + + 0.0722 * linear(pixel[2]) + + 0.05) + } + + fn target_rule_coverage(pixels: &[u8], width: usize, row: Rectangle) -> f32 { + // Only the lower padding, away from text, corners and scroll thumb. + // A pale fill or scattered text pixels cannot satisfy a continuous cue. + let left = (row.x + 14.).ceil() as usize; + let right = (row.x + row.width - 20.).floor() as usize; + let count = (left..right) + .filter(|x| { + ((row.y + row.height - 4.).ceil() as usize + ..(row.y + row.height - 1.).ceil() as usize) + .any(|y| { + let index = (y * width + x) * 4; + pixel_contrast_on_white(&pixels[index..index + 4]) >= 3. + }) + }) + .count(); + count as f32 / (right - left) as f32 + } + + #[tokio::test] + async fn mounted_keyboard_menu_paints_target_and_scroll_thumb_then_invokes_that_action() { + use iced::advanced::{overlay::Overlay as _, renderer::Headless}; + use iced_runtime::user_interface::{Cache, UserInterface}; + let mut renderer = ::new(iced::Font::DEFAULT, 13.into(), None) + .await + .expect("custom menu paint test requires WGPU headless rendering"); + let mut menu = menu(); + menu.anchor = Point::new(20., 20.); + menu.entries + .extend((30..70).map(|id| Entry::action(format!("Other action {id}"), id))); + let size = Size::new(480., 420.); + let mut measured_state = State::default(); + let mut measured = MenuOverlay { + menu: &menu, + state: &mut measured_state, + }; + let node = measured.layout(&renderer, size); + let geometry = measured.geometry(Layout::new(&node)); + let row = |index: usize| { + geometry.row( + menu.entries[..index].iter().map(Entry::height).sum(), + menu.entries[index].height(), + 0., + ) + }; + let thumb = geometry.scroll_thumb(0.).expect("fixture must overflow"); + let mut ui = UserInterface::build( + Popup::new( + iced::widget::Space::new() + .width(Length::Fill) + .height(Length::Fill), + Some(menu.clone()), + ), + size, + Cache::new(), + &mut renderer, + ); + let cursor = mouse::Cursor::Unavailable; + let mut messages = vec![]; + let theme = style::theme(); + let paint_style = renderer::Style { + text_color: style::INK, + }; + for (movement, active, inactive) in + [(None, 1, 4), (Some(keyboard::key::Named::ArrowDown), 4, 1)] + { + if let Some(movement) = movement { + let _ = ui.update( + &[key(movement)], + cursor, + &mut renderer, + &mut iced::advanced::clipboard::Null, + &mut messages, + ); + assert!( + messages.is_empty(), + "moving the target must not invoke an action" + ); + } + ui.draw(&mut renderer, &theme, &paint_style, cursor); + let pixels = Headless::screenshot(&mut renderer, Size::new(480, 420), 1., Color::WHITE); + assert!( + target_rule_coverage(&pixels, 480, row(active)) > 0.8, + "the keyboard target needs a continuous high-contrast shape cue" + ); + for index in [inactive, 2, 5] { + assert!( + target_rule_coverage(&pixels, 480, row(index)) < 0.2, + "ordinary and disabled rows must not acquire the target cue" + ); + } + let point = thumb.center(); + let index = (point.y.floor() as usize * 480 + point.x.floor() as usize) * 4; + assert!( + pixel_contrast_on_white(&pixels[index..index + 4]) >= 3., + "the actual overflow thumb must be visible without hover" + ); + } + let _ = ui.update( + &[ + key(keyboard::key::Named::Enter), + key(keyboard::key::Named::Enter), + ], + cursor, + &mut renderer, + &mut iced::advanced::clipboard::Null, + &mut messages, + ); + assert_eq!( + messages, + [22], + "Enter invokes exactly the painted Properties target once" + ); + } + #[test] fn actions_area_does_not_swallow_first_independent_click_after_overlay_consumed_its_release() { let release = Event::Mouse(mouse::Event::ButtonReleased(mouse::Button::Left)); diff --git a/crates/oa-desktop/src/forms/reference_chooser/trigger.rs b/crates/oa-desktop/src/forms/reference_chooser/trigger.rs index 68e0b314..733787f6 100644 --- a/crates/oa-desktop/src/forms/reference_chooser/trigger.rs +++ b/crates/oa-desktop/src/forms/reference_chooser/trigger.rs @@ -295,7 +295,7 @@ impl Widget for Field<'_, Mes color: if focus || cursor.is_over(layout.bounds()) { style::TEAL } else { - style::LINE + style::BOUNDARY }, width: if focus { 2. } else { 1. }, radius: 5.into(), diff --git a/crates/oa-desktop/src/forms/reference_chooser/widget.rs b/crates/oa-desktop/src/forms/reference_chooser/widget.rs index 112935dc..23c87341 100644 --- a/crates/oa-desktop/src/forms/reference_chooser/widget.rs +++ b/crates/oa-desktop/src/forms/reference_chooser/widget.rs @@ -708,7 +708,7 @@ impl> overlay::Overlay> overlay::Overlay 4. && row.height > 3. { + renderer.fill_quad( + renderer::Quad { + bounds: Rectangle { + x: row.x + 2., + y: row.y + row.height - 3., + width: row.width - 4., + height: 2., + }, + ..Default::default() + }, + style::TEAL, + ); + } } let alias_width = (row.width * 0.42).min( measure::( @@ -992,3 +1008,171 @@ fn raw_label>( bounds, ); } + +#[cfg(test)] +mod paint_tests { + use super::super::{Candidate, Context, Current, Outcome}; + use super::*; + use iced::advanced::renderer::Headless; + use iced_runtime::user_interface::{Cache, UserInterface}; + use oa_core::Id; + + fn named(key: keyboard::key::Named) -> Event { + Event::Keyboard(keyboard::Event::KeyPressed { + key: keyboard::Key::Named(key), + modified_key: keyboard::Key::Named(key), + physical_key: keyboard::key::Physical::Code(keyboard::key::Code::Space), + location: keyboard::Location::Standard, + modifiers: keyboard::Modifiers::empty(), + text: None, + repeat: false, + }) + } + + fn view(chooser: &Chooser, anchor: Rectangle) -> Element<'_, ChoiceEvent> { + Popup::new( + iced::widget::Space::new() + .width(Length::Fill) + .height(Length::Fill), + Some(chooser), + anchor, + std::convert::identity, + ) + .into() + } + + fn lower_rule_coverage(pixels: &[u8], row: Rectangle) -> f32 { + let linear = |byte: u8| { + let value = f32::from(byte) / 255.; + if value <= 0.04045 { + value / 12.92 + } else { + ((value + 0.055) / 1.055).powf(2.4) + } + }; + let left = (row.x + 14.).ceil() as usize; + let right = (row.x + row.width - 14.).floor() as usize; + let count = (left..right) + .filter(|x| { + ((row.y + row.height - 4.).ceil() as usize + ..(row.y + row.height - 1.).ceil() as usize) + .any(|y| { + let index = (y * 480 + x) * 4; + let pixel = &pixels[index..index + 4]; + let luminance = 0.2126 * linear(pixel[0]) + + 0.7152 * linear(pixel[1]) + + 0.0722 * linear(pixel[2]); + 1.05 / (luminance + 0.05) >= 3. + }) + }) + .count(); + count as f32 / (right - left) as f32 + } + + #[tokio::test] + async fn mounted_chooser_keyboard_target_is_visible_and_enter_keeps_exact_identity() { + let mut renderer = ::new(Font::DEFAULT, 13.into(), None) + .await + .expect("custom chooser paint test requires WGPU headless rendering"); + let context = Context::live( + Id::from_u128(1), + 7, + Id::from_u128(2), + Id::from_u128(3), + "/body/SequenceUse/sequence", + false, + ) + .unwrap(); + let mut chooser = Chooser::open( + context.clone(), + [(13, "Gamma"), (12, "Beta"), (11, "Alpha")] + .into_iter() + .map(|(id, name)| Candidate { + id: Id::from_u128(id), + name: name.into(), + alias: format!("candidate_{id}"), + kind: "Function".into(), + scope: "SA".into(), + containment: "System / supply topology".into(), + }) + .collect(), + Current::Unassigned, + "SA ยท compatible functions", + ) + .unwrap(); + let anchor = Rectangle { + x: 20., + y: 20., + width: 300., + height: 30., + }; + let size = Size::new(480., 420.); + let geometry = Geometry::new(anchor, size); + let cursor = mouse::Cursor::Unavailable; + let theme = style::theme(); + let paint_style = renderer::Style { + text_color: style::INK, + }; + let mut cache = Cache::new(); + let mut messages = vec![]; + for active in 0..2 { + let mut ui = UserInterface::build(view(&chooser, anchor), size, cache, &mut renderer); + ui.draw(&mut renderer, &theme, &paint_style, cursor); + let pixels = Headless::screenshot(&mut renderer, Size::new(480, 420), 1., Color::WHITE); + for index in 0..3 { + let coverage = lower_rule_coverage(&pixels, geometry.row(index)); + if index == active { + assert!( + coverage > 0.8, + "the actual keyboard candidate needs a continuous shape cue" + ); + } else { + assert!( + coverage < 0.2, + "the previous and ordinary candidates must not retain the cue" + ); + } + } + let events = if active == 0 { + vec![named(keyboard::key::Named::ArrowDown)] + } else { + vec![ + named(keyboard::key::Named::Enter), + named(keyboard::key::Named::Enter), + ] + }; + let _ = ui.update( + &events, + cursor, + &mut renderer, + &mut iced::advanced::clipboard::Null, + &mut messages, + ); + cache = ui.into_cache(); + assert_eq!( + messages.len(), + 1, + "one movement or one exact choice, with no repeated Enter" + ); + let outcome = chooser.update(messages.pop().unwrap()); + if active == 0 { + assert_eq!(outcome, Outcome::Browsed); + assert_eq!(chooser.highlighted().unwrap().id, Id::from_u128(12)); + assert_eq!( + chooser.current, + Current::Unassigned, + "navigation cannot write a reference" + ); + } else { + assert_eq!( + outcome, + Outcome::Write { + context: context.clone(), + value: Some(Id::from_u128(12)) + } + ); + assert!(!chooser.is_open()); + } + } + } +}