fix: keep suggestion popup within viewport - #4079
Open
escape0707 wants to merge 1 commit into
Open
Conversation
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.
Note
This pull request was developed with AI assistance. The submitter reviewed the implementation and tests before submission.
Summary
Design decisions
I considered measuring the rendered popup and then adjusting the real DOM. That would use the popup's exact width, but
.harper-containeris owned byvirtual-dom; changing it outside the VNode would allow the cached virtual tree and real DOM to disagree and require extra synchronization.I also prototyped a two-pass virtual-DOM approach: render a provisional left-aligned VNode, measure its real DOM, then create and patch a right-aligned VNode when necessary. That preserves virtual/real DOM consistency, but it adds another render and VNode construction and requires preserving child and hook identity to avoid unnecessary lifecycle work.
This PR instead uses the existing 420 px maximum width as a conservative pre-render estimate. It stays single-pass and declarative, with no layout measurement. A short popup near the right edge can be right-aligned even if its rendered content would have fit; that is an accepted tradeoff for the smaller implementation and lower runtime overhead.
Testing
pnpm exec biome check packages/lint-framework/src/lint/SuggestionBox.ts packages/chrome-plugin/tests/simple_textarea.spec.tspnpm --dir packages/lint-framework buildpnpm --dir packages/chrome-plugin zip-for-firefoxsimple_textarea.spec.ts: 24 passed, 6 skippedFixes #3972