Skip to content

Fix dropdown#17152

Merged
sdirix merged 1 commit intoeclipse-theia:masterfrom
ankitsharma101:fix-dropdown
Mar 18, 2026
Merged

Fix dropdown#17152
sdirix merged 1 commit intoeclipse-theia:masterfrom
ankitsharma101:fix-dropdown

Conversation

@ankitsharma101
Copy link
Copy Markdown
Contributor

What it does

Fixes the preference select dropdown staying open and mispositioned when the browser window is resized or when the Lumino split-panel sash is dragged (which shifts the layout but does not fire a standard window resize event).

Closes #12619

How to test

  1. Open Settings (File → Preferences → Open Settings UI)
  2. Search for editor.wordWrap
  3. Click the dropdown to open it
  4. Resize the browser window by dragging its edge → dropdown should close ✅
  5. Repeat steps 2-3, then drag the sidebar sash to shift the layout → dropdown should close ✅

Follow-ups

The fix uses (this.selectComponent.current as any).hide() to call the protected hide() method on SelectComponent without modifying the core widget. A potential follow-up would be to expose a public close() method on SelectComponent to avoid this cast. A technical debt issue could be created for this.

Breaking changes

  • This PR introduces breaking changes and requires careful review.

Attribution

Review checklist

  • As an author, I have thoroughly tested my changes and carefully followed the review guidelines
  • User-facing text is internationalized using the nls service (no user-facing text introduced in this PR)

Reminder for reviewers

  • As a reviewer, I agree to behave in accordance with the review guidelines

Copy link
Copy Markdown
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution ❤️

Comment thread packages/preferences/src/browser/views/components/preference-select-input.ts Outdated
Comment thread packages/preferences/src/browser/views/components/preference-select-input.ts Outdated
Comment thread packages/preferences/src/browser/views/components/preference-select-input.ts Outdated
Comment thread packages/preferences/src/browser/views/components/preference-select-input.ts Outdated
Comment thread packages/core/src/browser/style/tree.css
@github-project-automation github-project-automation Bot moved this from Waiting on reviewers to Waiting on author in PR Backlog Mar 12, 2026
@ankitsharma101
Copy link
Copy Markdown
Contributor Author

@sdirix Thank you for the detailed review and guidance! I have applied all of your suggestions:

Moved to Core: Added the resize listener into SelectComponent.attachListeners() so it applies globally. (I also added a quick instanceof Node check to the hide function so the event.target window resize doesn't trigger a contains() TypeError).

Lumino Update: Updated the ResizeObserver target in the preferences wrapper to use .lm-Widget. (Added a brief timeout to ensure the node is attached before querying the closest widget).

Removed Redundancies: Completely removed the custom pointerdown/mousedown capture listeners from the wrapper, relying entirely on the core onBlur behavior.

Coding Guidelines: Removed the _ prefix from the private resizeObserver property.

Synced with Master: Reset the branch to drop the tree.css commit since it is already in master.

I kept the ResizeObserver logic in the preferences wrapper because dragging the Lumino sash does not fire a global window.resize event, so the observer is still necessary to catch that specific layout shift.

Let me know if this looks good to go!

@ankitsharma101
Copy link
Copy Markdown
Contributor Author

ankitsharma101 commented Mar 16, 2026

Hi @sdirix, the linting error is fixed and the ResizeObserver logic is updated to observe the parent container directly. The CI is fully green!

Copy link
Copy Markdown
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Thanks for the update. I have one question and a comment. See comments.

Comment thread packages/core/src/browser/widgets/select-component.tsx Outdated
Comment thread packages/preferences/src/browser/views/components/preference-select-input.ts Outdated
@ankitsharma101 ankitsharma101 force-pushed the fix-dropdown branch 5 times, most recently from e34c812 to 198f011 Compare March 18, 2026 06:32
Comment thread packages/preferences/src/browser/views/components/preference-select-input.ts Outdated
@ankitsharma101 ankitsharma101 force-pushed the fix-dropdown branch 2 times, most recently from 05347d1 to 6041caa Compare March 18, 2026 09:08
@ankitsharma101
Copy link
Copy Markdown
Contributor Author

Hey @sdirix, I just removed that dispose() block!

Regarding the failing Playwright tests: I realized the new ResizeObserver was catching a micro layout shift the millisecond the dropdown opened, causing it to snap shut before the UI bot could click anything. >
To save you some back-and-forth, I went ahead and pushed a quick fix for this. I added a 2-pixel dimension check so the observer ignores the dropdown opening and only fires on an actual sash drag. Tested it locally and the UI feels bulletproof.

If you would rather handle this with a debounce instead, just say the word and I can swap it out in two seconds!

Signed-off-by: ankitsharma101 <ankitsharmaeclipse@gmail.com>
Copy link
Copy Markdown
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Works for me. Thanks!

@github-project-automation github-project-automation Bot moved this from Waiting on author to Needs merge in PR Backlog Mar 18, 2026
@sdirix sdirix merged commit 5820f1c into eclipse-theia:master Mar 18, 2026
10 checks passed
@github-project-automation github-project-automation Bot moved this from Needs merge to Done in PR Backlog Mar 18, 2026
@github-actions github-actions Bot added this to the 1.70.0 milestone Mar 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

[style] Selection box style error

2 participants