Skip to content

Refactored code to use Subscription.add#78

Merged
geoffreykwan merged 3 commits intodevelopfrom
issue-66-refactor-use-subscription-add
Apr 16, 2021
Merged

Refactored code to use Subscription.add#78
geoffreykwan merged 3 commits intodevelopfrom
issue-66-refactor-use-subscription-add

Conversation

@hirokiterashima
Copy link
Copy Markdown
Member

@hirokiterashima hirokiterashima commented Apr 14, 2021

Changes

  • For components that had multiple observable subscriptions, I replaced them by adding the subscription to a shared Subscription via subscription.add(), and unsubcribed all at once in ngOnDestry().
  • For components with single subscription, I left it the same.
  • Fixed some subscriptions that were never unsubscribed.

Additional changes

In those affected files, I also cleaned up the code:

  • removed unused dependencies in controllers
  • removed unnecessary variable declarations in controllers
  • removed unsubscribeAll() functions and instead call unsubscribe() directly
  • removed EditComponentController because it's no longer used

Test

  • Test all apps (student, teacher, author) and make sure that everything works as before. There is no new functionality, only refactored code.

Closes #66

For components that had multiple observable subscriptions, I replaced with a shared Subscription.
For components with single subscription, I left it the same.
Also:
- removed unused dependencies in controllers
- removed unnecessary variable declarations in controllers
- removed EditComponentController because it's no longer used
Copy link
Copy Markdown
Member

@geoffreykwan geoffreykwan left a comment

Choose a reason for hiding this comment

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

Looks good.

@geoffreykwan geoffreykwan merged commit 92efb27 into develop Apr 16, 2021
@geoffreykwan geoffreykwan deleted the issue-66-refactor-use-subscription-add branch April 16, 2021 20:39
hirokiterashima pushed a commit to encorelab/SCORE-Client that referenced this pull request Jun 28, 2021
…70-ta-next-step

WISE-Community#70 approve moves student to the next step
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor: use Subscription.add

2 participants