Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move SimpleFindWidget into base #155762

Open
Tyriar opened this issue Jul 20, 2022 · 2 comments
Open

Move SimpleFindWidget into base #155762

Tyriar opened this issue Jul 20, 2022 · 2 comments
Assignees
Labels
debt Code quality issues notebook-find terminal-find Relating the terminal's find widget
Milestone

Comments

@Tyriar
Copy link
Member

Tyriar commented Jul 20, 2022

Currently the new list view find box uses FindInput instead of SimpleFindWidget because the list lives in base, not workbench. We should give SimpleFindWidget the same treatment as HoverWidget to fix this. Ideally we would do this:

  • Move SimpleFindWidget into base
  • Allow injecting services into SimpleFindWidget for convenience when used in workbench (extending in platform?)
  • Have the editor FindWidget extend SimpleFindWidget to reduce code duplication

cc @rebornix @joaomoreno @andreamah

@Tyriar Tyriar added the debt Code quality issues label Jul 20, 2022
@Tyriar Tyriar added this to the Backlog milestone Jul 20, 2022
@Tyriar Tyriar self-assigned this Jul 20, 2022
@rebornix
Copy link
Member

rebornix commented Aug 1, 2022

👍 for moving the SimpleFindWidget to a lower layer, along with some polish for the input box #156179

Note that moving the find widget to base/platform would mean that it would be more challenging to bring in rich features to the input box. For example, we were considering supporting syntax highlighting for regex mode to improve the readability of the regular expressions. This can be done by replacing the input/texarea with a simple monaco editor. Another one is keybinding support for the input/texarea, users are expecting to use the same keybindings from the text editor, which can easily be supported by replacing the input/texarea with monaco. If this is what we want to have in the future, we may want to consdier how to make the input box swappable.

@Tyriar
Copy link
Member Author

Tyriar commented Dec 7, 2023

Passing this to just @andreamah as I doubt I'll end up doing this and it's closely related to #155761

@Tyriar Tyriar removed their assignment Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
debt Code quality issues notebook-find terminal-find Relating the terminal's find widget
Projects
None yet
Development

No branches or pull requests

3 participants