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

[Tree] Simplify synchronization #1100

Merged
merged 1 commit into from
Aug 5, 2016
Merged

[Tree] Simplify synchronization #1100

merged 1 commit into from
Aug 5, 2016

Conversation

VWoeltjen
Copy link
Contributor

Simplify synchronization of selection in tree with state passed-in, letting Angular deal with more of the specifics. Fixes #1008.

It's a little unclear why this was failing to begin with; the $watch on $parent wasn't firing when I'd expect it to. That said, the reason for doing this was essentially to replicate functionality already provided by Angular for synchronizing changes between an isolate scope and its parent. I'm comfortable with say "whatever was going on here, the Angular team has encountered and solved" instead of investing further time developing our own understanding.

There's a modest cost to this change, insofar as it introduces more watches (per tree, not per node). Think this cost is incidental.

This does not address any of the UX changes proposed for #1008; just fixes the originally reported bug as it currently manifests.

Author Checklist

  1. Changes address original issue? Y
  2. Unit tests included and/or updated with changes? Y
  3. Command line build passes? Y
  4. Changes have been smoke-tested? Y

Simplify synchronization of selection in tree with state
passed-in, letting Angular deal with more of the specifics.
Fixes #1008.
@larkin
Copy link
Contributor

larkin commented Aug 5, 2016

Reviewer Checklist

  1. Changes appear to address issue? Y
  2. Appropriate unit tests included? Y
  3. Code style and in-line documentation are appropriate? Y
  4. Commit messages meet standards? Y

@larkin larkin merged commit c8931f8 into master Aug 5, 2016
@larkin larkin deleted the tree-collapse-1008 branch August 5, 2016 23:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[locator widget] unclear what is happening when folders are not valid selections
3 participants