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

Refactor EventEmitter to using store observer #302

Open
louisgv opened this issue Jan 26, 2018 · 3 comments

Comments

@louisgv
Copy link
Contributor

commented Jan 26, 2018

Expected Behavior

With the store implemented, we should be able to use the value from it throughout the application and thus we can eliminate most EventEmitter implementation, since if there are communication cross component, it's best to do it once and use it everywhere.

Current Behavior

At the moment, the following can be refactored to consume store state:

  • ImageSeries - TreeView EvenEmitter when selecting a new image

Possible Solution

  • Compose a list of EventEmitter
  • Compose a list of store state that are nice to have
  • Refactor the EventEmitter into dispatcher and observer with the store.

Checklist before submitting

  • I have confirmed this using the officially supported Docker Compose setup using the local.yml configuration and ensured that I built the containers again and they reflect the most recent version of the project at the HEAD commit on the master branch
  • I have searched through the other currently open issues and am confident this is not a duplicate of an existing bug
  • I provided a minimal code snippet or list of steps that reproduces the bug.
  • I provided screenshots where appropriate
  • I filled out all the relevant sections of this template
@louisgv

This comment has been minimized.

Copy link
Contributor Author

commented Jan 26, 2018

After #288 I can start take a look at this.

@reubano

This comment has been minimized.

Copy link
Contributor

commented Jan 30, 2018

Great idea!

@reubano reubano added the enhancement label Jan 30, 2018

@louisgv

This comment has been minimized.

Copy link
Contributor Author

commented Jan 30, 2018

Since #288 is closed, I will start working on this issue. WIP PR will be issued soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.