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] Move row meta to CollapseTree #532

Merged
merged 1 commit into from
May 19, 2018

Conversation

pzuraq
Copy link
Contributor

@pzuraq pzuraq commented May 19, 2018

This PR refactors CollapseTree to take much more of the responsibility
of managing the state of rows, and setting up their meta objects. This
does a few things:

  • Normalizes the tree structures - CollapseTree now behaves much like
    ColumnTree. At some point they could probably be made to extend from
    the same base class.
  • Gives us enough context to correctly toggle selection (which allowed
    us to fix selection, and add the grouping mode)

API Changes

  • ember-tbody no longer accepts a tree object, it'll only take an
    array of rows. To get a single root node in a tree, pass in an array
    with a single parent node.
  • collapsed -> isCollapsed on nodes to bring it inline with
    terminology through the repo
  • New selectMode option: grouping
  • Toggling collapse state is now done by a checkbox is the first cell.
    Both checkboxes and the table itself will be styled to match Addepar's
    internal table usage in an upcoming PR.
  • Tree mode and collapsing are now toggleable via enableTree and
    enableCollapse on the tbody.

Other Stuff

  • Docs added for the tbody
  • Lots of new tests added!

Note: There is a subtle memory leak introduced in this PR, would like to fix it before we merge but this one is really tricky. I've narrowed it down to the onSelect action, somehow by sending rows up from there we're leaking.

@pzuraq pzuraq force-pushed the pzuraq/bugfix/tree-selection branch from f565859 to 9aeeddf Compare May 19, 2018 16:53
This PR refactors CollapseTree to take much more of the responsibility
of managing the state of rows, and setting up their meta objects. This
does a few things:

* Normalizes the tree structures - CollapseTree now behaves much like
  ColumnTree. At some point they could probably be made to extend from
  the same base class.
* Gives us enough context to correctly toggle selection (which allowed
  us to fix selection, and add the `grouping` mode)

* `ember-tbody` no longer accepts a `tree` object, it'll only take an
  array of rows. To get a single root node in a tree, pass in an array
  with a single parent node.
* `collapsed` -> `isCollapsed` on nodes to bring it inline with
  terminology through the repo
* New `selectMode` option: `grouping`
* Toggling collapse state is now done by a checkbox is the first cell.
  Both checkboxes and the table itself will be styled to match Addepar's
  internal table usage in an upcoming PR.
* Tree mode and collapsing are now toggleable via `enableTree` and
  `enableCollapse` on the `tbody`.

* Docs added for the `tbody`
* Lots of new tests added!
@pzuraq pzuraq force-pushed the pzuraq/bugfix/tree-selection branch from 9aeeddf to 4fea608 Compare May 19, 2018 18:19
@pzuraq pzuraq merged commit 285a5f1 into master May 19, 2018
@pzuraq pzuraq deleted the pzuraq/bugfix/tree-selection branch May 19, 2018 19:22
@addepar-andy
Copy link
Contributor

Toggling collapse state is now done by a checkbox is the first cell.

why a checkbox? that isn't how the analysis table works

@pzuraq
Copy link
Contributor Author

pzuraq commented May 21, 2018

We can style a checkbox to look just like the toggle icon that exists in our internal tables today. Using a checkbox is more semantically correct and gives us a lot of behavior and a11y for free.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants