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

fix(schematics): tree schematic not working #12281

Conversation

devversion
Copy link
Member

@devversion devversion commented Jul 19, 2018

  • Fixes that the new tree schematic is not working when added to an Angular app. Since one FileNode does not have the required type property, the TS compiler will complain about wrongly assigning the example data to the DataSource.

  • Also makes it more obvious what interface is doing. Right now, at first glance, I was pretty much confused by TreeNode and FileNode and what the flattener is actually doing.

    It should be a easy-to-use and easy-to-understand quick-start schematic.

Actual error in a new NG app
image

@devversion devversion requested a review from amcdnl as a code owner July 19, 2018 13:24
@googlebot googlebot added the cla: yes PR author has agreed to Google's Contributor License Agreement label Jul 19, 2018
return node.expandable;
};

/** Get whether the node has children or not. */
hasChild(index: number, node: FlatTreeNode){
Copy link
Contributor

Choose a reason for hiding this comment

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

can we have a space before {?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. Note that I just moved that already existing code one block up.

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 aside from that one space, add merge-ready when ready

@jelbourn jelbourn added pr: lgtm target: patch This PR is targeted for the next patch release and removed target: master only labels Jul 20, 2018
* Fixes that the new `tree` schematic is not working when added to an Angular app. Since one `FileNode` does not have the required `type` property, the TS compiler will complain about wrongly assigning the example data to the `DataSource`.
@devversion devversion force-pushed the fix/schematics-tree-schematic-not-working branch from 45e067f to b700c4e Compare July 20, 2018 15:02
@devversion devversion added target: master only and removed target: patch This PR is targeted for the next patch release labels Jul 20, 2018
@devversion
Copy link
Member Author

@jelbourn Done. I think it should be master only because the tree schematic has been merged a few days ago into master only as well.

@devversion devversion added the action: merge The PR is ready for merge by the caretaker label Jul 20, 2018
@jelbourn
Copy link
Member

Ah, yeah then. For some reason I thought it had been there for a while.

@josephperrott josephperrott merged commit ba134f4 into angular:master Jul 20, 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 9, 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.

6 participants