-
Notifications
You must be signed in to change notification settings - Fork 79
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
[WIP] feat: add demo for tree-view-type-ahead #39
Conversation
@Shijir, your company's legal contact has approved your signed contributor license agreement. It will also be reviewed by VMware, but the merge can proceed. |
Signed-off-by: stsogoo <[email protected]>
e7f5e93
to
99850e8
Compare
👋 @Shijir,
Thank you, 🤖 Clarity Release Bot |
@@ -188,6 +199,20 @@ export class ClrTreeNode<T> implements OnInit, OnDestroy { | |||
this.subscriptions.push( | |||
this._model.loading$.pipe(debounceTime(0)).subscribe(isLoading => (this.isModelLoading = isLoading)) | |||
); | |||
|
|||
this.subscriptions.push( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the takeUntil/destroy
pattern in this PR
@@ -99,6 +99,71 @@ export class TreeFocusManagerService<T> { | |||
} | |||
} | |||
|
|||
private findNodeStartsWith(searchString: string, model: TreeNodeModel<T>): TreeNodeModel<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the return type should be TreeNodeModel<T> | null
return null; | ||
} | ||
|
||
private findClosestNodeStartsWith(searchString: string, model: TreeNodeModel<T>): TreeNodeModel<T> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the return type should be TreeNodeModel<T> | null
return null; | ||
} | ||
|
||
// Look from its own descendents first |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are lots of chunks of logic in these private functions that might be easier to unit test in isolation as utilities or helpers
Signed-off-by: stsogoo <[email protected]>
Signed-off-by: stsogoo <[email protected]>
Closing this PR as a topic branch PR is created instead: #61 |
Hi there 👋, this is an automated message. To help Clarity keep track of discussions, we automatically lock closed PRs after 14 days. Please look for another open issue or open a new issue with updated details and reference this one as necessary. |
Signed-off-by: stsogoo [email protected]
PR Checklist
Please check if your PR fulfills the following requirements:
Enable Type-Ahead functionaly Tree-View by introducing
clrTypeAhead
directive.clrTypeAhead
directive works as a support/helper directive to enable a type-ahead focus functionality in Tree-View component. Its only goal is to extract the text content from its host element. This directive can extract and provide element's text content in two ways:.textContent
from its native element. Example:In this way, the user directly provides the text through input binding. This is better because the directive doesn't need to access the element to extract its text content.
In this current implementation, Type-Ahead functionality is optional in Tree-View as not all Tree-View components need this functionality. Thus, in order to enable this functionality, users need to import
ClrTypeAheadModule
together withClrTreeViewModule
.Recursive Tree-View with Type-Ahead:
Declaritive Tree-View with Type-Ahead:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: VPAT-604: Tree Views lack Type-ahead functionality https://jira.eng.vmware.com/plugins/servlet/samlsso?redirectTo=%2Fbrowse%2FVPAT-604
What is the new behavior?
Does this PR introduce a breaking change?
Other information