Skip to content

IgxTree Implementation #8945

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 42 commits into from
Apr 19, 2021
Merged

IgxTree Implementation #8945

merged 42 commits into from
Apr 19, 2021

Conversation

ViktorSlavov
Copy link
Contributor

@ViktorSlavov ViktorSlavov commented Feb 9, 2021 •
edited by kacheshmarova
Loading

Closes #7475
Closes #9092
IgxTree Implementation

Spec

  • Basic Implementation (file structure, dev sample)
    • IgxTreeComponent
    • IgxTreeNodeComponent
    • Dev sample
  • Node state implementation
    • Tree API service
    • Tree state methods (expand/collapse/toggle)
    • Tree node input (expanded)
    • Explore refactoring base IgxExpansionPanel class to reuse of animation code
    • Animations (open/close animation as in IgxExpansionPanel)
    • Expand/Collapsed events
  • Selection
    • Selection Modes ('None', 'BiState', 'Cascading')
    • Cascading Selection - Implementation
    • Tree API methods (selectAll/deselectAll node)
    • Tree node input (selected)
    • Selection events
  • Keyboard Navigation
    • Basic navigation (active item)
    • Selection
    • Toggle
  • Aria Support
    • Aria roles and tab index on tree / tree nodes
    • igxTreeNodeLink directive + roles
  • Styling
    • Tree and Tree Node classes in theming
    • Actual stying
  • Testing
    • Test scenarios in spec
    • Implement unit tests
    • Implement e2e tests
  • Docs
    • README.md
    • docfx page entry
    • igniteui-angular-sample entry
    • igniteui-cli template
  • Drag and drop
    • Basic implementation
    • Events and event args

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@kdinev kdinev added the tree label Feb 10, 2021
@kacheshmarova kacheshmarova assigned mmart1n and unassigned martinesoff Feb 18, 2021
@kdinev
Copy link
Member

kdinev commented Feb 19, 2021

@ViktorSlavov Although it's a little bit late for test-driven, my suggestion is to catch up on unit tests now and to implement the rest with a test-driven approach!

@ViktorSlavov ViktorSlavov mentioned this pull request Mar 1, 2021
Closed
@ViktorSlavov ViktorSlavov requested a review from kdinev March 18, 2021 14:07
@ViktorSlavov ViktorSlavov added 🛠️ status: in-development Issues and PRs with active development on them version: 11.1.x labels Mar 18, 2021
kdinev
kdinev reviewed Mar 25, 2021
View reviewed changes
projects/igniteui-angular/src/lib/tree/tree-node/tree-node.component.ts Outdated
* const children: IgxTreeNode<any>[] = node.children;
* ```
*/
public get _children(): IgxTreeNode<any>[] {
Copy link
Member

Choose a reason for hiding this comment

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

Should the public getter be with _?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be change once all functionality is implemented - we'll remove the _ and swap this w/ the current children: QueryList<IgxTreeNode<any>> property. I've left a TODO comment above the API docs for this one.
There's currently a children: QueryList<IgxTreeNode> property, which is /** @hidden @internal */ and is used in the logic for selection and navigation.

projects/igniteui-angular/src/lib/tree/tree-node/tree-node.component.ts
* ```
*/
@Input()
public get expanded() {
Copy link
Member

Choose a reason for hiding this comment

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

Also you need expandedChange for [(expanded)] binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My mistake, I've added that

projects/igniteui-angular/src/lib/tree/tree-node/tree-node.component.ts
* ```
*/
@Input()
public get selected(): boolean {
Copy link
Member

Choose a reason for hiding this comment

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

Also you need selectedChange for [(selected)] binding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

One was implemented w/ the selection service, it's at the start of the class definition (to avoid making lint angry)

kdinev
kdinev previously approved these changes Mar 25, 2021
View reviewed changes
@simeonoff simeonoff self-requested a review April 7, 2021 07:23
@ViktorSlavov ViktorSlavov requested a review from kdinev April 16, 2021 14:15
simeonoff
simeonoff previously approved these changes Apr 16, 2021
View reviewed changes
@ViktorSlavov ViktorSlavov added ❌ status: awaiting-test PRs awaiting manual verification and removed 🛠️ status: in-development Issues and PRs with active development on them labels Apr 19, 2021
@tishko0 tishko0 added 💥 status: in-test PRs currently being tested and removed ❌ status: awaiting-test PRs awaiting manual verification labels Apr 19, 2021
@tishko0 tishko0 added ✅ status: verified Applies to PRs that have passed manual verification and removed 💥 status: in-test PRs currently being tested labels Apr 19, 2021
kdinev
kdinev reviewed Apr 19, 2021
View reviewed changes
projects/igniteui-angular/src/lib/tree/common.ts Outdated
}

// Enums
export const IGX_TREE_SELECTION_TYPE = mkenum({
Copy link
Member

Choose a reason for hiding this comment

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

Make this pascal-case please

kdinev
kdinev previously approved these changes Apr 19, 2021
View reviewed changes
kdinev
kdinev previously approved these changes Apr 19, 2021
View reviewed changes
@Lipata Lipata requested review from wnvko and simeonoff April 19, 2021 11:53
@Lipata Lipata requested a review from kdinev April 19, 2021 12:32
@kdinev kdinev merged commit 5a9e304 into master Apr 19, 2021
@Lipata Lipata deleted the vslavov/tree-POC branch April 19, 2021 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kdinev kdinev kdinev approved these changes

@simeonoff simeonoff simeonoff approved these changes

@wnvko wnvko wnvko approved these changes

Assignees

@tishko0 tishko0

@ViktorSlavov ViktorSlavov

@mmart1n mmart1n

@MonikaKirkova MonikaKirkova

@teodosiah teodosiah

Labels
tree version: 12.0.x ✅ status: verified Applies to PRs that have passed manual verification
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

IgxTree - Styling Include igTree from jQuery to Ignite UI for Angular