Skip to content

Conversation

@ndoschek
Copy link
Member

What it does

Resolves GH-16657

  • Refactor SearchBox to a ReactWidget for improved state handling
  • Update icons in the search box to use Codicons
  • Update and simplify CSS rules

How to test

  • Test the tree search box in different tree views, e.g. in explorer, outline, scm view
  • Verify the behaviour described in the issue is fixed

Follow-ups

Breaking changes

  • This PR introduces breaking changes and requires careful review. If yes, the breaking changes section in the changelog has been updated.

Attribution

Review checklist

Reminder for reviewers

Resolves GH-16657

- Refactor SearchBox to a ReactWidget for improved state handling
- Update icons in the search box to use Codicons
- Update and simplify CSS rules
@github-project-automation github-project-automation bot moved this to Waiting on reviewers in PR Backlog Dec 16, 2025
@ndoschek ndoschek requested a review from lucas-koehler January 8, 2026 15:09
Copy link
Contributor

@lucas-koehler lucas-koehler 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 fix @ndoschek . The changes look pretty good to me and the search box still worked as expected using it in the explorer and scm views. I could not detect any overflow :)

Just one question. Why were the constants for the style classes (i.e. SearchBox.Styles) removed? The old approach seemed quite nice to me considering some of the classes are used multiple times.

@ndoschek
Copy link
Member Author

ndoschek commented Jan 9, 2026

Thanks for the review @lucas-koehler!
There wasn't a strong technical reason for removing the constants. I mainly found it a bit easier to read with everything in one React component now, especially when conditional classes are involved. For example:

className={`${SearchBox.Styles.SEARCH_BUTTON} ${SearchBox.Styles.ACTION_LABEL} ${this.isFiltering ? SearchBox.Styles.LIST_FILTER_ICON : SearchBox.Styles.LIST_SELECTION_ICON}`}

instead of

className={`theia-search-button action-label ${this.isFiltering ? codicon('list-filter') : codicon('list-selection')}`}

That said, I agree the constants can be beneficial however. I'm happy to revert this or adjust the approach if you insist or have a better idea for this.

Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

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

@ndoschek Thanks for the explanation. No, I do not insist on reverting this. Both approaches are fine for me with different (dis)advantantages. We can merge it like this

@github-project-automation github-project-automation bot moved this from Waiting on reviewers to Needs merge in PR Backlog Jan 9, 2026
@ndoschek ndoschek merged commit 04fc7df into master Jan 9, 2026
16 of 17 checks passed
@ndoschek ndoschek deleted the GH-16657 branch January 9, 2026 13:57
@github-project-automation github-project-automation bot moved this from Needs merge to Done in PR Backlog Jan 9, 2026
@github-actions github-actions bot added this to the 1.68.0 milestone Jan 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Tree search box is overflowing

3 participants