Skip to content

Replace browse combo wrappers with ComboBox editors#268

Draft
cj-radcliff wants to merge 4 commits intomainfrom
combo-box-reset
Draft

Replace browse combo wrappers with ComboBox editors#268
cj-radcliff wants to merge 4 commits intomainfrom
combo-box-reset

Conversation

@cj-radcliff
Copy link
Collaborator

@cj-radcliff cj-radcliff commented Feb 24, 2026

This code attempts to replace the deprecated ComboboxWithBrowseButton. It was entirely coded with AI and reviewed by AI. It should be reviewed carefully by a human prior to accepting.

@cj-radcliff
Copy link
Collaborator Author

/gemini review please

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively replaces the deprecated ComboboxWithBrowseButton with a standard ComboBox that uses a custom ExtendableTextField editor. This is a good modernization of the UI components, and the changes are applied consistently across several configuration dialogs. I've identified a minor issue where an incorrect resource bundle key is used for a tooltip, which could lead to user confusion. My suggested fix addresses this. Overall, this is a solid refactoring.

@cj-radcliff
Copy link
Collaborator Author

/gemini Review the latest fix that should fix your last review comment

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

The pull request successfully replaces the deprecated ComboboxWithBrowseButton with modern ComboBox editors using ExtendableTextField. This improves UI consistency and adheres to current IntelliJ platform standards. However, there is a regression in DartRemoteDebugConfigurationEditor where manual path entry is no longer correctly handled, and the display text in the editable field now includes presentable names instead of raw paths.

@cj-radcliff
Copy link
Collaborator Author

/gemini review latest

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request refactors several UI components, replacing ComboboxWithBrowseButton with a modern ComboBox that uses an ExtendableTextField as its editor. The changes are well-executed across multiple files, including UI forms and their corresponding Java classes. The new implementation correctly replicates the old functionality, such as the browse-for-folder action and dynamic content loading. I've found a couple of minor issues that could improve the robustness and consistency of the implementation.

…erver/ui/DartRemoteDebugConfigurationEditor.java

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@cj-radcliff
Copy link
Collaborator Author

/gemini can you look one more time?

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request successfully replaces the deprecated ComboboxWithBrowseButton with modern ComboBox editors across various UI components. The implementation is clean and follows current IntelliJ Platform UI best practices. I've found one area for improvement to ensure UI consistency, which is detailed in the review comment.

@cj-radcliff
Copy link
Collaborator Author

cj-radcliff commented Feb 24, 2026

Hi @pq : comprehensive reviewer note for this PR.

Why this change exists

The primary goal is to replace ComboboxWithBrowseButton usage with modern ComboBox-based editable controls because the previous API path is deprecated/removed in the current IntelliJ platform target and was causing compile failures (BasicComboBoxEditor/UI integration issues in this branch?s migration context).

Scope summary

PR #268 updates 8 files (Java + .form + bundle strings):

  • DartRemoteDebugConfigurationEditor (+form)
  • DartSdkUtil
  • DartConfigurable (+form)
  • DartGeneratorPeer (+form)
  • DartBundle.properties

Net effect: replace wrapper browse-combo widgets with plain ComboBox<T> plus ExtendableTextField browse extension, and keep behavior equivalent where possible.

File-by-file details

  1. DartSdkUtil.java
  • initDartSdkControls now accepts ComboBox<String> instead of ComboboxWithBrowseButton.
  • Browse action is implemented via ExtendableTextField.Extension + FileChooser.chooseFile(...).
  • Keeps existing SDK path heuristics (.../dart-sdk fallback) and known-path model population.
  • Version label refresh now occurs on any document text change event (not only INSERT).
  1. DartConfigurable.java + .form
  • UI field type changed to ComboBox<String>.
  • Call sites switched from getComboBox().getEditor() to getEditor().
  • Reset/model update logic kept functionally equivalent.
  1. DartGeneratorPeer.java + .form
  • Same migration pattern as DartConfigurable: now ComboBox<String> and direct editor access.
  • Validation/template loading still reads from editor text, behavior preserved.
  1. DartRemoteDebugConfigurationEditor.java + .form
  • ComboboxWithBrowseButton replaced with ComboBox<NameAndPath> (custom-created in form).
  • Browse action moved to ExtendableTextField extension.
  • Added dedicated message key for project-path chooser title/tooltip:
    • button.browse.dialog.title.select.dart.project.path
  • Model typing fixed (NameAndPath[]::new) to avoid generic inference issues.
  • Important follow-up fixes from review:
    • Manual typed input is now preserved in applyEditorTo (instead of becoming empty when selected item is not NameAndPath).
    • NameAndPath.toString() now returns raw path (myPath) so editable text remains path-centric, while renderer still shows presentable Name (path) in dropdown.
  1. DartBundle.properties
  • Added button.browse.dialog.title.select.dart.project.path=Select Dart Project Path.
  • No functional changes beyond message key addition.

Reviewer-facing behavior changes to double-check

  1. Remote Debug editor text semantics changed intentionally:
  • Dropdown rows: still presentable text.
  • Editable field value when selected/typed: now raw path.
  1. SDK path version label now updates on all document changes (insert/delete/replace), not insert-only.

  2. Browse button rendering is now extension-based icon inside text field, not wrapper button component.

  • This may differ slightly in spacing/focus/keyboard behavior depending on LAF.

Potential risks / edge cases

  1. Path normalization for manually typed remote-debug path:
  • applyEditorTo stores typed text with .trim().
  • It does not force FileUtil.toSystemIndependentName(...) for manual text.
  • Likely acceptable, but worth validating on Windows-style \ path input because downstream path comparisons/use may assume / form in some places.
  1. UX parity risk from component swap:
  • Since this is a component-level migration, regressions are most likely around focus traversal, accessibility tooltip behavior, and keyboard interaction (Enter/arrow/selection commit) rather than business logic.
  1. Form custom-create dependency:
  • .form files now rely on custom-create="true" for ComboBox fields. If UI designer regeneration touches these forms, verify bindings still instantiate expected component types.

Validation performed

  • Local compile check repeatedly passed after final fixes:
    • gradlew.bat compileJava
  • PR CI at time of writing:
    • Passing: scan-pr, build-plugin, unit-tests (ubuntu/windows), verify-plugin, cla/google.
    • One run still in progress/rolling when checked: unit-tests (macos-latest).

Suggested manual spot checks (high value)

  1. Dart Settings page:
  • Type SDK path manually (valid + invalid), ensure version label updates correctly.
  • Use browse icon and verify chosen path populates editor.
  1. New Dart Project wizard:
  • Manual SDK typing, browse selection, validation messaging, template loading refresh.
  1. Remote Debug config:
  • Select existing project entry vs manually type project path.
  • Reopen config and ensure path round-trips without being blanked.
  • Verify dropdown presentation remains friendly while editor text remains editable path.

If you want, I can follow up by adding a focused regression test around remote-debug path round-tripping for typed string input (non-NameAndPath selected item).

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant