Skip to content

Conversation

tinayuangao
Copy link
Contributor

No description provided.

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Nov 30, 2017
@tinayuangao tinayuangao added pr: needs review and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels Nov 30, 2017

getChildren = (node: JsonNode) => { return ofObservable(node.children); }

hasChild = (_: number, _nodeData: JsonFlatNode) => { return _nodeData.expandable; }

Choose a reason for hiding this comment

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

@tinayuangao I noticed this a week or so ago while trialling the tree; users have to define two functions that achieve the exact same result but with different signatures; isExpandable, used by the when, and hasChild, used by the TreeControl. Would it be sensible to change this?

/** Tree flattener to transfrom JsonNode to JsonFlatNode */
export class TreeFlattener {

static flattenNodes( structuredData: JsonNode[]): JsonFlatNode[] {
Copy link
Member

Choose a reason for hiding this comment

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

Extra space

structuredData.forEach((node) => {
this._flattenNode(node, 0, resultNodes, []);
});
return resultNodes;
Copy link
Member

Choose a reason for hiding this comment

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

return structuredData.map(node => this._flattenNode(node, 0, resultNodes, []));

/** Tree flattener to transfrom JsonNode to JsonFlatNode */
export class TreeFlattener {

static flattenNodes( structuredData: JsonNode[]): JsonFlatNode[] {
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to just have module-level functions instead of static methods?

<mat-card>
<mat-card-header>Flattened tree</mat-card-header>

<mat-tree [dataSource]="dataSource" [treeControl]="treeControl">-->
Copy link
Member

Choose a reason for hiding this comment

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

Hanging --> ?


constructor(database: JsonDatabase) {
// For flat tree
this.treeControl = new FlatTreeControl<JsonFlatNode>(this.getLevel, this.isExpandable);
Copy link
Member

Choose a reason for hiding this comment

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

Would it make sense to use a more real-world example, such as collection of directories?

@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Dec 9, 2017
@tinayuangao
Copy link
Contributor Author

Comments addressed. Please take a look. Thanks!

Copy link
Member

@jelbourn jelbourn left a comment

Choose a reason for hiding this comment

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

LGTM

@jelbourn jelbourn added pr: lgtm action: merge The PR is ready for merge by the caretaker labels Dec 13, 2017
@tinayuangao tinayuangao merged commit 7a5f335 into angular:tree Dec 13, 2017
tinayuangao added a commit that referenced this pull request Dec 13, 2017
tinayuangao added a commit that referenced this pull request Dec 14, 2017
tinayuangao added a commit that referenced this pull request Dec 15, 2017
tinayuangao added a commit to tinayuangao/material2 that referenced this pull request Dec 15, 2017
tinayuangao added a commit that referenced this pull request Dec 19, 2017
tinayuangao added a commit that referenced this pull request Jan 4, 2018
tinayuangao added a commit that referenced this pull request Jan 10, 2018
tinayuangao added a commit that referenced this pull request Jan 23, 2018
tinayuangao added a commit that referenced this pull request Feb 5, 2018
tinayuangao added a commit that referenced this pull request Feb 9, 2018
jelbourn pushed a commit that referenced this pull request Feb 12, 2018
tinayuangao added a commit that referenced this pull request Feb 13, 2018
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes PR author has agreed to Google's Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants