fix: reset themed cursor when sidecar type is null or omitted - #3
fix: reset themed cursor when sidecar type is null or omitted#3My-Denia wants to merge 3 commits into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
The new CursorPane preview test does not pin the locale despite asserting English-only labels, making it prone to environment/order-dependent failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes cursor-type reset behavior in the compositor when cursorType is null or omitted in the cursor sidecar, and improves the cursor theme picker preview so themes with distinct arrow/pointer artwork are represented accurately.
Changes:
- Treat missing/
nullcursorTypesamples as a reset to"arrow"inCursorTrack::load, with a regression test. - Add
themePickerPreviewAssets()and update the Cursor pane theme grid to optionally show both arrow and pointer preview sprites. - Add unit/UI tests covering theme preview asset selection and the Cursor pane preview rendering.
File summaries
| File | Description |
|---|---|
| src/lib/cursor/cursorThemes.ts | Adds themePickerPreviewAssets() helper to determine arrow/pointer preview asset paths. |
| src/lib/cursor/cursorThemes.test.ts | Adds unit tests for themePickerPreviewAssets() behavior across default, arrow-only, and a known dual-sprite theme. |
| src/components/ai-edition/RightPanes.tsx | Updates cursor theme options to use multiple preview URLs and renders 1–2 preview images per theme cell. |
| src/components/ai-edition/NewEditorShell.module.css | Adds layout styling for the multiple-preview container in cursor theme cells. |
| src/components/ai-edition/CursorPane.preview.test.tsx | Adds a jsdom UI test asserting CursorPane theme cells render 1 vs 2 preview images. |
| crates/compositor/src/cursor.rs | Resets semantic cursor type to "arrow" when cursorType is null/absent; adds a regression test. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| beforeEach(() => { | ||
| stubStorage(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| cleanup(); | ||
| }); |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Superseded by the upstream pull request getopenscreen#566 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b42e00f4d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let ct = match s.get("cursorType") { | ||
| Some(v) => v.as_str().filter(|label| !label.is_empty()).unwrap_or("arrow"), | ||
| None => "arrow", | ||
| }; |
There was a problem hiding this comment.
Because this changes cursor-state selection used by native compositor preview/export, the repository requires a cursor-focused manual E2E slice and a results-log row identifying the build, platform, and skipped coverage; I checked technical-documentation/testing/manual-e2e-checklist.md, and there is no entry for this commit. Run the relevant native cursor preview/export checks on an available real platform and append the result, or log the checks as skipped with the environment reason, before treating the cross-platform behavior as verified.
AGENTS.md reference: AGENTS.md:L86-L90
Useful? React with 👍 / 👎.
Summary
Cursor sidecar samples that store cursorType as JSON null, or omit the key, now reset to arrow instead of holding the last pointer or text state. Theme picker cells show both arrow and pointer when a pack has distinct art, so a pack is not previewed as arrow-only.
Related issue
Themed cursor remaining on pointer after a null or omitted type. This fork PR does not use a GitHub closing keyword.
Type of change
Release impact
Desktop impact
Screenshots / video
Theme picker: Hello Kitty cell shows arrow and pointer; Default shows a single sprite.
Testing