Skip to content

[VSCodeSweeper MVP — test only] Can't hover on top few pixels of editor#2

Draft
egamma wants to merge 1 commit into
mainfrom
vscodesweeper/fix-188878
Draft

[VSCodeSweeper MVP — test only] Can't hover on top few pixels of editor#2
egamma wants to merge 1 commit into
mainfrom
vscodesweeper/fix-188878

Conversation

@egamma

@egamma egamma commented Jul 7, 2026

Copy link
Copy Markdown
Owner

⚠️ VSCodeSweeper MVP — test only. This draft PR was generated by an AI experiment
(vscodesweeper) inside a personal fork. It is
not intended for microsoft/vscode and is not a contribution proposal.

Upstream issue: microsoft#188878

Change summary

Hovering over text in the top ~6 pixels of the editor viewport is blocked by the scroll-decoration overlay element.

Validation

Open an editor, scroll so the first line is near the top, and verify that hovering over text in the top 6px still triggers the text cursor and editor hover tooltip (not the pointer cursor). Alternatively, add a test verifying the scroll-decoration element has pointer-events: none in its computed style.


Branch vscodesweeper/fix-188878 · generated fix · review loop runs separately (codex review).

@egamma egamma added the vscodesweeper-mvp VSCodeSweeper MVP test PR (fork-only) label Jul 7, 2026
@egamma

egamma commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

🤖 copilot /review — round 1

VSCodeSweeper MVP self-review (fork-only experiment).

● Get diff stats (shell)
│ cd && git diff main...HEAD --stat
└ 4 lines…

● Get commits on branch (shell)
│ cd && git log main...HEAD --oneline
└ 2 lines…

● Get full diff (shell)
│ cd && git diff main...HEAD
└ 42 lines…

Good, the CSS file is present in the out/ directory. Now let me check the import path in the test - it uses '../../../base/common/path.js' which from src/vs/editor/test/node/ goes up 3 levels to src/vs/ then down. But the test is at src/vs/editor/test/node/scrollDecoration.test.ts — let me count:

  • ..src/vs/editor/test/
  • ../..src/vs/editor/
  • ../../..src/vs/

So '../../../base/common/path.js' resolves to src/vs/base/common/path.js. That's correct.

Now compare to fixtures.test.ts at src/vs/editor/test/node/diffing/fixtures.test.ts which uses '../../../../base/common/path.js' (4 levels up because it's one directory deeper). The new test's import path looks correct.

The test itself is essentially just a "canary" test that reads a CSS file and checks it contains a specific string. It's not testing behavior, just verifying file content hasn't regressed. While this is a questionable testing pattern (it's basically a snapshot of one line), the user's question is about bugs and logic errors, not test philosophy.

Let me verify the CSS change itself is correct and complete:The CSS fix is correct and straightforward. The pointer-events: none on an absolutely-positioned decorative overlay is the standard fix.

No significant issues found in the reviewed changes.

The CSS fix is correct — pointer-events: none is the appropriate solution for a decorative overlay intercepting mouse events. The test, while arguably low-value (it's essentially asserting a file's content hasn't changed, which version control already ensures), has no bugs — the FileAccess.asFileUri pattern and import paths match the existing fixtures.test.ts convention, and the CSS file is confirmed present in the out/ directory at runtime.● Code-review(claude-opus-4.6) Review scroll decoration fix
│ Check if CSS is in out dir (shell)
│ │ ls
│ │ out/vs/editor/browser/viewParts/scrollDecorat
│ ion/
│ │ 2>/dev/null || echo "DIRECTORY NOT FOUND"
│ └ 3 lines…
└ Read scrollDecoration.css
│ src/vs/editor/browser/viewParts/scrollDecoration/scrollDecoration.css
└ 13 lines read

Review Summary

CSS fix (scrollDecoration.css): ✅ Correct and appropriate. pointer-events: none is the standard solution for decorative overlays that shouldn't intercept mouse events.

Test (scrollDecoration.test.ts): No bugs, but low value — it simply reads the CSS file and asserts the string pointer-events: none exists. This is the kind of invariant that version control and code review already enforce. If the line were accidentally removed, a regression test exercising actual pointer/hover behavior would catch it more meaningfully. That said, it's not harmful.

No high-confidence bugs or logic errors found.

@egamma

egamma commented Jul 7, 2026

Copy link
Copy Markdown
Owner Author

✅ Round 1: no actionable findings — the review loop converged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vscodesweeper-mvp VSCodeSweeper MVP test PR (fork-only)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant