fix: prevent find widget from overlapping sticky scroll when editor is narrow#320290
Closed
Mr-Ahmadi wants to merge 1 commit into
Closed
fix: prevent find widget from overlapping sticky scroll when editor is narrow#320290Mr-Ahmadi wants to merge 1 commit into
Mr-Ahmadi wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds sticky scroll awareness to the editor Find widget so it can offset itself when sticky scroll is visible.
Changes:
- Retrieve the sticky scroll controller contribution and track its height changes
- Store sticky scroll height and apply a top offset to the Find widget DOM node
- Refresh the offset when the widget is revealed
Comment on lines
+246
to
+253
| const stickyScrollController = this._codeEditor.getContribution<IStickyScrollController>(StickyScrollController.ID); | ||
| if (stickyScrollController) { | ||
| this._stickyScrollHeight = stickyScrollController.stickyScrollWidgetHeight; | ||
| this._register(stickyScrollController.onDidChangeStickyScrollHeight(({ height }) => { | ||
| this._stickyScrollHeight = height; | ||
| this._updateStickyScrollOffset(); | ||
| })); | ||
| } |
Comment on lines
+559
to
+565
| private _updateStickyScrollOffset(): void { | ||
| if (this._stickyScrollHeight > 0) { | ||
| this._domNode.style.marginTop = `${this._stickyScrollHeight + 4}px`; | ||
| } else { | ||
| this._domNode.style.marginTop = ''; | ||
| } | ||
| } |
| import * as strings from '../../../../base/common/strings.js'; | ||
| import './findWidget.css'; | ||
| import { ICodeEditor, IOverlayWidget, IOverlayWidgetPosition, IViewZone, OverlayWidgetPositionPreference } from '../../../browser/editorBrowser.js'; | ||
| import { IStickyScrollController, StickyScrollController } from '../../stickyScroll/browser/stickyScrollController.js'; |
Mr-Ahmadi
force-pushed
the
fix/find-widget-sticky-scroll-overlap
branch
from
3a70f94 to
587c58a
Compare
Author
|
@microsoft-github-policy-service agree |
Mr-Ahmadi
force-pushed
the
fix/find-widget-sticky-scroll-overlap
branch
from
587c58a to
ae56928
Compare
Mr-Ahmadi
added a commit
to Mr-Ahmadi/vscode
that referenced
this pull request
Jun 8, 2026
…0290) The find widget uses OverlayWidgetPositionPreference.TOP_RIGHT_CORNER which positions it at top: 0 of the editor. When sticky scroll is visible, the find widget renders on top of it due to a higher z-index (35 vs 4). Shifts the find widget down by the sticky scroll height via margin-top when sticky scroll is present, preventing overlap. Uses StickyScrollController.get() to retrieve the controller and subscribes to onDidChangeStickyScrollHeight for dynamic updates. Re-queries in _reveal() if the controller wasn't yet available at construction time (registred AfterFirstRender). Closes microsoft#316851
Author
|
Updated the fix with improvements over the initial version:
|
…0290) The find widget uses OverlayWidgetPositionPreference.TOP_RIGHT_CORNER which positions it at top: 0 of the editor. When sticky scroll is visible, the find widget renders on top of it due to a higher z-index (35 vs 4). Shifts the find widget down by the sticky scroll height via margin-top when sticky scroll is present, preventing overlap. Uses StickyScrollController.get() to retrieve the controller and subscribes to onDidChangeStickyScrollHeight for dynamic updates. Re-queries in _reveal() if the controller wasn't yet available at construction time (registred AfterFirstRender). Closes microsoft#316851
Mr-Ahmadi
force-pushed
the
fix/find-widget-sticky-scroll-overlap
branch
from
ae56928 to
79f0210
Compare
Author
|
Closing this PR — pivoting to a different fix. |
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
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.
The find widget uses
OverlayWidgetPositionPreference.TOP_RIGHT_CORNERwhich always positions attop: 0of the editor viewport. When sticky scroll is visible, it renders on top of the sticky scroll content due to the find widget's higherz-index(35 vs 4).This is especially noticeable when the editor window is narrowed (e.g., split-screen layout), where the find widget can encroach and partially obscure the sticky scroll function names.
Closes
Fix
Listens to the sticky scroll controller for height changes and shifts the find widget down accordingly via
margin-top:StickyScrollController.get()(the canonical VS Code pattern) instead of a magic string IDmargin-top = stickyScrollHeight + 4px(4px is the base CSS margin, extracted asFIND_WIDGET_TOP_MARGIN)margin-top: 4px_reveal()if unavailable at construction time (defensive, since it's registeredAfterFirstRender)Changes
FIND_WIDGET_TOP_MARGIN— named constant for the base CSS margin-top_stickyScrollInitialized/_stickyScrollHeight— tracks setup state and current height_initStickyScroll()— retrieves the controller viaStickyScrollController.get(), subscribes toonDidChangeStickyScrollHeight, and records initial height_updateStickyScrollOffset()— applies or clears the inlinemargin-topbased on_stickyScrollHeightVerification
transform) unaffected_reveal()