Skip to content

IgxTree Implementation#8945

Merged
kdinev merged 42 commits intomasterfrom
vslavov/tree-POC
Apr 19, 2021
Merged

IgxTree Implementation#8945
kdinev merged 42 commits intomasterfrom
vslavov/tree-POC

Conversation

@ViktorSlavov
Copy link
Contributor

@ViktorSlavov ViktorSlavov commented Feb 9, 2021

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
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
@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
* 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.

* ```
*/
@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

* ```
*/
@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
@ViktorSlavov ViktorSlavov requested a review from kdinev April 16, 2021 14:15
simeonoff
simeonoff previously approved these changes Apr 16, 2021
@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
}

// 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
kdinev
kdinev previously approved these changes Apr 19, 2021
@Lipata Lipata requested review from simeonoff and wnvko April 19, 2021 11:53
@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

Labels

tree version: 12.0.x ✅ status: verified Applies to PRs that have passed manual verification

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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