list: support accessible names on row containers - #5
Closed
grishy wants to merge 10 commits into
Closed
Conversation
## Description App Stories cards inherited prose list markers and spacing, making the catalog tall and difficult to scan. Scope the page styles and use the GPUI Kit website's typography, neutral theme tokens, compact controls, and consistent card spacing. Screenshot previews fill a shared 16:9 area, with author, platform/date, and icon-only links organized into aligned rows. Featured now displays six apps per page in editorial order. All apps displays nine entries per page from the complete catalog, defaults to newest publication first, and owns its search, category filters, and star sorting. Search, category, and sort changes reset All apps to page one. Empty results keep the controls available. Star counts use compact notation, accessible names, and an optical alignment correction; Commercial precedes the count, and the separate Read story row is removed. Replace the native sort menu with a theme-aware keyboard-operable listbox and use 32px search/sort controls. Normalize navigation action label sizing and centered line boxes. Update the website design rules and test both pagination sizes and boundaries. Implementation and tests were AI-assisted, with iterative visual feedback and independent code review. Use `bunx` to invoke Astro binaries directly in the package scripts. The previous `bun --bun astro` invocation resolved the identically named package script recursively in CI and failed with E2BIG. Verified the standard `bun run build` command after this fix. Align Test Docs with the release workflow by checking out the approved Showcase catalog, installing its locked dependencies with Bun 1.4.0, and passing `SHOWCASES_DIR`. Run catalog and SEO tests in the website check. ## Visual verification Checked English and Chinese, light and dark themes, and widths from 360 to 1440 pixels in Chromium. Verified aligned card metadata and icon rows, edge-to-edge previews, keyboard focus, selection, empty-state recovery, reduced motion, and Featured's 6/6/4 pagination and All apps' 9/9/9/9/3 pagination, with no duplicate entries. The App Stories content also fits at 200% root font size. The existing shared site navigation overflows in that text-only scaling case; this pre-existing text-scaling overflow remains outside the toolbar alignment fix. ## How to Test From `website/`: ```sh bun run test:showcases bun run build bun run test:seo ``` Open `/apps` and `/zh-CN/apps`. Change Featured pages, search and clear an unmatched query, combine search with a category, and switch between publication and star sorting. Featured selection and order should remain independent of All apps filters. ## Checklist - [x] Read and followed the contributing guidelines. - [x] Reviewed the changes and ran the website tests and build. - [ ] Maintainer review of AI-assisted changes. Rust Story and platform performance tests are not applicable to this website-only change.
…ongbridge#2965) ## Description `website/src/components/GitHubStar.vue` and `website/src/components/SidebarFilter.vue` have had no references since the VitePress to Astro migration in longbridge#2926. `Nav.astro` renders the GitHub star link with inline markup, and no page or component mounts a sidebar filter, so both files were dead source. The other seven `.vue` files under `website/src/components/` are each mounted by a page and are left untouched. Astro supports Vue islands through the official `@astrojs/vue` integration, which the migration adopted deliberately, so their presence is not a leftover. Removing `SidebarFilter.vue` records the current state rather than changing it: the sidebar filter was already absent from the rendered site, and documentation search is served by Pagefind through `DocSearch.astro`. Restoring a sidebar filter would be a separate feature change. Identified and removed with AI assistance. ## How to Test From `website/`: ```sh grep -rn "GitHubStar\|SidebarFilter" src tests astro.config.mjs package.json bun run build ``` The grep should return nothing. The build should complete with 305 pages. The site's appearance and behavior are unchanged, since neither file was reachable. ## Checklist - [x] Read and followed the contributing guidelines. - [x] Verified no references remain and ran the website build. - [ ] Maintainer review of AI-assisted changes. No documentation, locale, or public API changes. Rust tests are not applicable to this website-only change. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01RwSD6p7jLbJEF8hYtrhth4 Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
## Description Fix Next/Previous advancing the search counter without revealing the match after manual scrolling. Search navigation now scrolls in either direction while preserving configured surrounding-line padding; cursor navigation keeps its existing direction restrictions. ## Screenshot Before https://github.com/user-attachments/assets/d71cc59a-5b62-4dd7-a837-5380b2ee2395 After https://github.com/user-attachments/assets/2f43770a-edf7-4dbd-9188-e68b3a32efe9 ## Validation - Added regression tests for Next scrolling upward and Previous scrolling downward, including surrounding-line padding. - `cargo test -p gpui-base input:: --lib --offline` — 152 tests passed locally. AI assistance: Codex generated the implementation and regression tests. ## Checklist - [x] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document and followed the guidelines. - [ ] Reviewed the changes in this PR and confirmed AI generated code (If any) is accurate. - [ ] Passed `cargo run` for story tests related to the changes. - [x] Tested macOS, Windows and Linux platforms performance (if the change is platform-specific) — Not applicable; the search navigation fix is platform-independent.
I use GPUI Kit in NearWord (app based on gpui-kit now). These changes
come from my fork, that I made during testing/
`test-support` enables GPUI's test harness but leaves out the native
platform implementation. On macOS, screenshot capture can fail with
`render_to_image not implemented for this platform`. This patch also
enables `gpui_platform/test-support`.
It adds `profiler = ["gpui/profiler"]` so applications can enable
profiling through `gpui-kit`, without adding a direct GPUI dependency.
Profiling stays opt-in. The patch includes feature documentation and a
compile-time `WindowProfiler` export check.
Default features, dependency versions, and the lockfile are unchanged.
## How to Test
Passed on macOS:
- Build checks with default features, no default features, and
`test-support` and `profiler` separately and together.
- Kit tests, doctests, formatting, and Clippy with warnings denied.
- Isolated consumer checks for platform test support and opt-in
profiling.
- Native Metal capture, with the center pixel matching `[51, 102, 153,
255]`.
<details>
<summary>Build and test commands</summary>
Run from the repository root. All commands should pass. The export check
runs at compile time.
```sh
cargo check -p gpui-kit --locked
cargo check -p gpui-kit --no-default-features --locked
cargo check -p gpui-kit --features test-support --locked
cargo check -p gpui-kit --features profiler --locked
cargo check -p gpui-kit --features test-support,profiler --locked
cargo test -p gpui-kit --features test-support,profiler --locked
cargo clippy -p gpui-kit --all-targets --features test-support,profiler --locked -- -D warnings
```
</details>
<details>
<summary>Reproduce native screenshot capture on macOS</summary>
On macOS with Metal, save this as
`crates/kit/examples/native_render.rs`; create the directory if needed.
This temporary probe is not part of the committed tests.
```rust
use gpui_kit::{AppContext as _, Context, IntoElement, Render, Styled, VisualTestAppContext, div, px, size};
struct Swatch;
impl Render for Swatch {
fn render(&mut self, _: &mut gpui_kit::Window, _: &mut Context<Self>) -> impl IntoElement {
div().size_full().bg(gpui_kit::rgb(0x336699))
}
}
fn main() {
let mut cx = VisualTestAppContext::new(gpui_kit::platform::current_platform(false));
let window = cx.open_offscreen_window(size(px(64.), px(48.)), |_, cx| cx.new(|_| Swatch))
.expect("open native offscreen window");
cx.run_until_parked();
let image = cx.capture_screenshot(window.into()).expect("native render_to_image");
let pixel = image.get_pixel(image.width()/2, image.height()/2);
assert_eq!(pixel.0, [0x33, 0x66, 0x99, 0xff]);
println!("native rendering passed: {}x{}, center={:?}", image.width(), image.height(), pixel.0);
}
```
From the repository root, run:
```sh
cargo run -p gpui-kit --example native_render --features test-support --locked
```
Capture and the pixel assertion should succeed. Observed output:
```text
native rendering passed: 128x96, center=[51, 102, 153, 255]
```
Image dimensions depend on the display scale; the center pixel must
match. Remove the temporary example after testing:
```sh
rm crates/kit/examples/native_render.rs
```
</details>
Story was not used for this feature-flag change; the native probe checks
screenshot capture directly. Windows and Linux runtime checks were not
run.
## Checklist
- [x] Read
[CONTRIBUTING.md](https://github.com/longbridge/gpui-kit/blob/main/CONTRIBUTING.md).
- [x] Reviewed the patch, including AI-assisted changes.
- [x] Manually tested the patch.
- [ ] Windows and Linux runtime validation.
## AI assistance
OpenAI’s `gpt-6-astra` extracted these changes from my GPUI Kit fork
used in NearWord and prepared this PR. I reviewed the patch and made
edits before submitting it.
---------
Co-authored-by: Jason Lee <huacnlee@gmail.com>
`gpui-base` declares `gpui_platform` for every non-wasm target, but only the examples use it. For targets that supply their own backend through `Application::with_platform` (iOS, Android), the build fails inside `gpui_platform`. Following what `examples/wasm` already does, this moves the native showcase and motion examples into their own package, `gpui-base-examples`, which includes the same shared code with `#[path]`; the `gpui-base` manifest no longer mentions `gpui_platform`. `src/bin/components.rs` and `examples/components.rs` were two entry points to the same showcase and become the package's `components` bin; the README commands are updated accordingly. Verified: - `cargo check -p gpui-base-examples --bins` - `cargo check -p gpui-base --examples --tests --benches` - `cargo check -p gpui-base --target aarch64-apple-ios-sim` and `--target wasm32-unknown-unknown` - `cargo check --workspace` - `cargo fmt --all --check` Closes longbridge#2962 --------- Co-authored-by: Jason Lee <huacnlee@gmail.com>
## Description Preserve the active editor search occurrence when Find is closed and reopened with the same query. An unchanged query now keeps the matcher’s current index and is not re-anchored to the viewport. New queries and case-sensitivity changes still rebuild and anchor the search normally. This also prevents the styled search panel’s initial query synchronization from resetting the occurrence. ## Screenshot Before https://github.com/user-attachments/assets/4f334ada-27f8-4a17-8ad3-5b768aff058b After https://github.com/user-attachments/assets/3ef1f5ae-1c56-4829-9f49-87513ef5a039 ## How to test 1. Search for a term with at least three matches. 2. Navigate to the second match and close Find. 3. Reopen Find without changing the query. 4. Confirm the counter remains at `2/3` and Next advances to `3/3`. ```sh cargo test -p gpui-base input::search::tests cargo test -p gpui-component input::overlay::tests ``` AI assistance: Codex generated the implementation and regression tests; I reviewed changes. ## Checklist - [x] I have read the [CONTRIBUTING](../CONTRIBUTING.md) document and followed the guidelines. - [ ] Reviewed the changes in this PR and confirmed AI-generated code (if any) is accurate. - [ ] Passed `cargo run` for story tests related to the changes. - [x] Platform-specific performance testing is not applicable; this is platform-independent search state synchronization.
Owner
Author
|
Opened upstream as longbridge#2977 with the reviewed patch and description. Closing this review PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
List owns the container that represents each selectable row. A delegate can render text, icons, and other content inside it, but cannot give that outer container an explicit accessible name.
This adds the optional
ListDelegate::item_accessibility_labelhook and applies its result to the existing row container. Applications can describe a composite row as one item, rather than relying on the names of its individual children. The List Story example combines the company name, price, and price change.The default returns
None, so existing delegates need no changes. The patch does not hide children, change the visible layout, or replace selection and activation handling.Accessibility check
Captured from a company row in the macOS Story example:
{ "role": "AXGroup", "title": "Grimes and Pouros LLC, 171.29, -19.90%", "actions": [ "AXPick", "AXPress" ] }The name is on the same node as the row's selection and activation actions. The agent verified that
AXPressselects the named row and that names remain available after changing the query.The AX capture verifies the metadata. I also manually checked row navigation and announcements with VoiceOver on macOS.
How to Test
On macOS:
AXGrouptitle should contain the company, price, and price change.AXPress. The row should become selected.The Story search and row-measurement bugs are separate fixes in #2976 and #2975. This PR does not fix stale search results or overlapping rows.
The regression checks the row container without opening a native window:
cargo test -p gpui-component --lib accessible_names_belong_to_the_row_container --lockedIt checks both an explicit name and
None, while preserving theListItemrole.Automated checks
cargo test -p gpui-base -p gpui-component --lib --locked cargo clippy -p gpui-base -p gpui-component -p gpui-component-story --all-targets --locked -- -D warnings771 base and 419 component tests passed, along with strict Clippy, formatting, and the diff whitespace check.
Windows and Linux runtime were not tested.
Checklist
AI assistance
I used this change in my application, then asked OpenAI's
gpt-6-astrato extract it into a standalone PR. The agent prepared the patch, regression test, and description, and ran the automated checks. I reviewed the extracted patch and manually tested it again on macOS, including VoiceOver.