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

[find/replace overlay] Separate UI from Controller Logic #1912

Open
Tracked by #2021
Wittmaxi opened this issue May 31, 2024 · 2 comments
Open
Tracked by #2021

[find/replace overlay] Separate UI from Controller Logic #1912

Wittmaxi opened this issue May 31, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@Wittmaxi
Copy link

Wittmaxi commented May 31, 2024

This Issue Documents a review-comment from the find/replace overlay PR
which could not be addressed into the current proposal
comment by @HeikoKlare

I see some options for improving the design of this class, but I would leave this for follow-up work as this class is internal, so can be adapted easily, and I don't want to defer a merge of this functionality. Things I would consider to make the class more maintainable are at least:

- Separate the UI setup from the controller logic
- Maybe extract layouting functionality (which parts of the UI to show and where to place them) into a separate class
- Update UI elements via listeners on the FindReplaceLogicState rather than making interactions with one UI element change the enablement of another
@Wittmaxi Wittmaxi added the enhancement New feature or request label May 31, 2024
@laeubi
Copy link
Contributor

laeubi commented May 31, 2024

@Wittmaxi using the eclipse sideffects is a quite powerful way to address the

Update UI elements via listeners on the FindReplaceLogicState rather than making interactions with one UI element change the enablement of another

Basically it works like this:

  1. Use WidgetSideEffects to create a factory
  2. Use WidgetProperties to wrap interesting properties you like to observe into IObvervables
  3. now create a side-effect and instead of query e.g. the checked state of the widget use the IObvervable equivalent and update the control you like accordingly

That way as soon as anything you have accessed will update it immediately without further thinking about listeners and states.

@Wittmaxi
Copy link
Author

@laeubi thank you for the advice! That indeed sounds very smart :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants