From 3d87be93b86d9f919805dda6f38d6f52c6f7d924 Mon Sep 17 00:00:00 2001 From: owjs3901 Date: Fri, 2 Oct 2026 17:35:57 +0900 Subject: [PATCH 1/2] fix(components): make Select an accessible listbox with keyboard support Refs #692 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- .changepacks/changepack_log_select_a11y.json | 7 + bindings/devup-ui-wasm/src/lib.rs | 8 +- libs/css/src/theme_tokens.rs | 2 +- libs/extractor/src/lib.rs | 8 +- libs/extractor/src/tailwind.rs | 4 +- .../__snapshots__/index.browser.test.tsx.snap | 30 +-- .../Select/__tests__/index.browser.test.tsx | 192 +++++++++++++++--- .../src/components/Select/index.tsx | 102 ++++++++-- packages/components/src/contexts/useSelect.ts | 1 + 9 files changed, 286 insertions(+), 68 deletions(-) create mode 100644 .changepacks/changepack_log_select_a11y.json diff --git a/.changepacks/changepack_log_select_a11y.json b/.changepacks/changepack_log_select_a11y.json new file mode 100644 index 000000000..594b6596a --- /dev/null +++ b/.changepacks/changepack_log_select_a11y.json @@ -0,0 +1,7 @@ +{ + "changes": { + "packages/components/package.json": "Patch" + }, + "note": "Select follows the listbox pattern: the trigger has aria-haspopup, aria-controls and opens with ArrowDown/ArrowUp; the popup is a listbox (aria-multiselectable for checkbox selects) whose options carry role=option, aria-selected and aria-disabled; opening focuses the selected or first option, arrows/Home/End move between enabled options, Enter/Space choose one, and Escape or choosing closes the popup and returns focus to the trigger. An outside click on a controlled select now calls onOpenChange(false) instead of changing hidden internal state", + "date": "2026-10-01T00:00:00.000Z" +} diff --git a/bindings/devup-ui-wasm/src/lib.rs b/bindings/devup-ui-wasm/src/lib.rs index 4560410f3..0a2347ac2 100644 --- a/bindings/devup-ui-wasm/src/lib.rs +++ b/bindings/devup-ui-wasm/src/lib.rs @@ -1551,7 +1551,7 @@ mod tests { ); // Test getters - assert!(!output.code().is_empty()); + assert_ne!(output.code(), ""); assert_eq!(output.css_file(), Some("devup-ui-0.css".to_string())); assert_eq!(output.map(), Some("//# sourceMappingURL=test".to_string())); assert!(output.css().is_some()); @@ -1951,7 +1951,7 @@ mod tests { assert!(result.is_ok()); let output = result.unwrap(); - assert!(!output.code().is_empty()); + assert_ne!(output.code(), ""); assert!(output.map().is_some()); } @@ -1975,7 +1975,7 @@ mod tests { assert!(result.is_ok()); let output = result.unwrap(); - assert!(!output.code().is_empty()); + assert_ne!(output.code(), ""); assert!(output.map().is_none()); } @@ -2000,7 +2000,7 @@ mod tests { assert!(result.is_err()); if let Err(error) = result { - assert!(!error.is_empty()); + assert_ne!(error, ""); } } diff --git a/libs/css/src/theme_tokens.rs b/libs/css/src/theme_tokens.rs index d8d8fb021..3b2346aed 100644 --- a/libs/css/src/theme_tokens.rs +++ b/libs/css/src/theme_tokens.rs @@ -119,7 +119,7 @@ mod tests { set_typography_keys(vec!["body".to_string(), "title".to_string()]); assert_eq!(get_typography_keys(), vec!["body", "title"]); set_typography_keys(vec![]); - assert!(get_typography_keys().is_empty()); + assert_eq!(get_typography_keys(), Vec::::new()); } #[test] diff --git a/libs/extractor/src/lib.rs b/libs/extractor/src/lib.rs index 68981f36c..96c6ca430 100644 --- a/libs/extractor/src/lib.rs +++ b/libs/extractor/src/lib.rs @@ -836,8 +836,8 @@ mod tests { alternate: None, }; - assert!(empty.extract().is_empty()); - assert!(empty.into_extract().is_empty()); + assert_eq!(empty.extract(), vec![]); + assert_eq!(empty.into_extract(), vec![]); } #[test] @@ -13598,7 +13598,7 @@ globalCss({ ); assert!(result.is_ok()); let output = result.unwrap(); - assert!(!output.code.is_empty()); + assert_ne!(output.code, ""); } #[test] @@ -18778,7 +18778,7 @@ export const k = styled('div')({ color: SIZE });", &memory_resolver(CONSTANT_MODULES), ) .unwrap(); - assert!(without_imports.dependencies.is_empty()); + assert_eq!(without_imports.dependencies.len(), 0); let without_constants = extract_with_modules( "/src/Handler.tsx", "import { Box } from '@devup-ui/react';\nimport { handler } from './handler';\nexport const a = ;", diff --git a/libs/extractor/src/tailwind.rs b/libs/extractor/src/tailwind.rs index 87e9f5178..70643aa7c 100644 --- a/libs/extractor/src/tailwind.rs +++ b/libs/extractor/src/tailwind.rs @@ -284,7 +284,7 @@ pub struct TailwindClass { /// non-overlapping) but mutates the existing buffer instead of allocating a new /// `String`. `needle` must be non-empty. fn remove_all_substr(haystack: &mut String, needle: &str) { - debug_assert!(!needle.is_empty()); + debug_assert_ne!(needle, ""); let mut search_from = 0; while let Some(rel) = haystack[search_from..].find(needle) { let at = search_from + rel; @@ -3934,7 +3934,7 @@ mod tests { #[test] fn test_empty_string() { let styles = parse_tailwind_to_styles(""); - assert!(styles.is_empty()); + assert_eq!(styles, vec![]); } #[test] diff --git a/packages/components/src/components/Select/__tests__/__snapshots__/index.browser.test.tsx.snap b/packages/components/src/components/Select/__tests__/__snapshots__/index.browser.test.tsx.snap index 4e6276f64..5f33abd4d 100644 --- a/packages/components/src/components/Select/__tests__/__snapshots__/index.browser.test.tsx.snap +++ b/packages/components/src/components/Select/__tests__/__snapshots__/index.browser.test.tsx.snap @@ -2,8 +2,8 @@ exports[`Select should render 1`] = ` "
-
- -
-
+
+
Option 1
-
+
Option 2
-
+
Option 3
-
+
Option 4
-
-