/h

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

Fix exception when GROUPS_CHANGED event is broadcast during route load #2688

Merged
merged 1 commit into from Oct 29, 2015

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@robertknight @nickstenning
@robertknight
Contributor

robertknight commented Oct 29, 2015

The directive attempted to update itself in
response to a group change notification without triggering
a full digest cycle by using $scope.$apply.

This was based on the incorrect understanding that $apply only
dirty-checks the current scope downwards. In fact, in dirty-checks
the root scope. Additionally, the logic was pointless since
group list/focus changes happen in response to two types of events,
both of which are triggered in the context of $apply:

  • An event handler when the user selects a group
  • A callback from angular-websocket when a WebSocket message
    is received.
@robertknight
Fix exception when GROUPS_CHANGED event is broadcast during route load 
The <group-list> directive attempted to update itself in
response to a group change notification without triggering
a full digest cycle by using `$scope.$apply`.

This was based on the incorrect understanding that $apply only
dirty-checks the current scope downwards. In fact, in dirty-checks
the root scope. Additionally, the logic was pointless since
group list/focus changes happen in response to two types of events,
both of which are triggered in the context of $apply:

 * An event handler when the user selects a group

 * A callback from angular-websocket when a WebSocket message
   is received.
12be399
@nickstenning

This comment has been minimized.

Show comment
Hide comment
@nickstenning

nickstenning Oct 29, 2015

Contributor

LGTM.
Contributor

nickstenning commented Oct 29, 2015

LGTM.

nickstenning added a commit that referenced this pull request Oct 29, 2015

@nickstenning
Merge pull request #2688 from robertknight/groups_changed_exception 
Fix exception when GROUPS_CHANGED event is broadcast during route load
65e3985

@nickstenning nickstenning merged commit 65e3985 into hypothesis:master Oct 29, 2015
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment