Skip to content

fix(cursor): define cursor styles so it can change#2154

Merged
aidanhb merged 3 commits intomainfrom
aidan/cursor-styles
Mar 24, 2026
Merged

fix(cursor): define cursor styles so it can change#2154
aidanhb merged 3 commits intomainfrom
aidan/cursor-styles

Conversation

@aidanhb
Copy link
Copy Markdown
Contributor

@aidanhb aidanhb commented Mar 24, 2026

Allows cursor to change based on styling. Fixes bug where cursor wasn't changing on resize gutter hover.

@aidanhb aidanhb requested a review from a team as a code owner March 24, 2026 18:47
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 24, 2026

Walkthrough

The pull request adds 14 CSS custom properties to the :root selector in js/app/packages/app/index.css: --cursor-auto, --cursor-default, --cursor-pointer, --cursor-text, --cursor-move, --cursor-grab, --cursor-grabbing, --cursor-not-allowed, --cursor-col-resize, --cursor-row-resize, --cursor-ew-resize, --cursor-ns-resize, --cursor-nwse-resize, and --cursor-nesw-resize. Each property is initialized with a corresponding browser cursor keyword, providing overridable global variables for cursor styling. No other selectors or rules were changed.

Poem

🐰 I hopped into :root with a twitchy nose,
Fourteen cursors lined up like tiny toes,
Pointer, grab, and resize dance in play,
I nibble pixels and frolic through the day,
CSS carrots gleam — a rabbit’s cursor show! 🥕

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title directly describes the main change: defining cursor styles to enable cursor changes, which matches the CSS custom properties added to control cursor styling.
Description check ✅ Passed The description is directly related to the changeset, explaining the purpose of the cursor style definitions and the specific bug fix (resize gutter hover issue).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch aidan/cursor-styles

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 24, 2026

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/app/packages/app/index.css`:
- Around line 260-272: SelectionRenderer.tsx uses var(--cursor-nwse-resize) and
var(--cursor-nesw-resize) but those CSS custom properties are missing; update
the CSS variables block in index.css (the cursor variables section) to declare
--cursor-nwse-resize and --cursor-nesw-resize with values "nwse-resize" and
"nesw-resize" respectively so the diagonal resize cursors are applied
consistently across the app.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: cae26c79-53a1-44b4-8511-4751f2e736c8

📥 Commits

Reviewing files that changed from the base of the PR and between 66b62f6 and fdd6516.

📒 Files selected for processing (1)
  • js/app/packages/app/index.css

/* Cursor variables — override these to use custom cursors */
--cursor-auto: auto;
--cursor-default: default;
--cursor-pointer: pointer;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we make this "default" default for now. would be a decent way of forcing the constant claude slop "cursor-pointer" to not show pointer for now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@js/app/packages/app/index.css`:
- Line 263: The CSS variable --cursor-pointer is set to default (a temporary
override) which mismatches its name; update the declaration for --cursor-pointer
in index.css by adding a concise inline comment explaining this is an
intentional temporary pointer->default mapping and why, and optionally rename
the variable to a clearer name (e.g., --cursor-default-override or
--cursor-pointer-disabled) and update all usages of --cursor-pointer accordingly
so the intent is explicit to future maintainers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 02ddba2e-1da4-4770-b841-1f30f394f4a2

📥 Commits

Reviewing files that changed from the base of the PR and between 2b9fcb7 and 0519dea.

📒 Files selected for processing (1)
  • js/app/packages/app/index.css

/* Cursor variables — override these to use custom cursors */
--cursor-auto: auto;
--cursor-default: default;
--cursor-pointer: default;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Document the temporary pointer-to-default mapping.

The --cursor-pointer variable is set to default instead of pointer, creating a semantic mismatch between the variable name and its value. While this appears intentional as a temporary workaround, it could confuse future developers who expect var(--cursor-pointer) to display a pointer cursor.

📝 Suggested documentation

Add an inline comment explaining the temporary nature of this mapping:

- --cursor-pointer: default;
+ --cursor-pointer: default; /* TODO: Temporary - prevents unwanted pointer cursors in generated code */

Alternatively, consider using a more explicit variable name like --cursor-default-override or --cursor-pointer-disabled to make the intention clearer.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
--cursor-pointer: default;
--cursor-pointer: default; /* TODO: Temporary - prevents unwanted pointer cursors in generated code */
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@js/app/packages/app/index.css` at line 263, The CSS variable --cursor-pointer
is set to default (a temporary override) which mismatches its name; update the
declaration for --cursor-pointer in index.css by adding a concise inline comment
explaining this is an intentional temporary pointer->default mapping and why,
and optionally rename the variable to a clearer name (e.g.,
--cursor-default-override or --cursor-pointer-disabled) and update all usages of
--cursor-pointer accordingly so the intent is explicit to future maintainers.

@aidanhb aidanhb merged commit b2d2b4a into main Mar 24, 2026
26 checks passed
@aidanhb aidanhb deleted the aidan/cursor-styles branch March 24, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants