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

Tree: If user filter tree nodes and then lazy load new children (with .key prop) - nodeClick event is not emitted #14723

Closed
kievsash opened this issue Feb 6, 2024 · 8 comments
Assignees
Labels
LTS-PORTABLE Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Milestone

Comments

@kievsash
Copy link

kievsash commented Feb 6, 2024

Describe the bug

Primeng tree - seems like all version affected

Reproduce steps:

  1. user just used a p-tree filter
  2. then lazyLoaded some additional nodes -
  3. onNodeClick do not emit event on newly added nodes click

Root cause:
treeRef.value is not refering same node objects as treeRef.filteredNodes anymore after filtering (https://github.com/primefaces/primeng/blob/16.9.1/src/app/components/tree/tree.ts#L1617)
And since if we have .key prop on node objects - in onNodeClick event Node is checked for existence on treeRef.value object and not in treeRef.filteredNodes
https://github.com/primefaces/primeng/blob/16.9.1/src/app/components/tree/tree.ts#L1247

So newly added nodes after filtering dot not emit onNodeClick event.

Solution:
If add node.children after filtering add them to respective node in treeRef.value as well
Maybe not create node copy for filteredNodes but use original node object?

Reproduce repo: https://codesandbox.io/p/devbox/primeng-tree-lazy-demo-forked-hvm6ls

Environment

https://codesandbox.io/p/devbox/primeng-tree-lazy-demo-forked-hvm6ls

Reproducer

No response

Angular version

16.2.0

PrimeNG version

16.9.4-lts

Build / Runtime

Angular CLI App

Language

TypeScript

Node version (for AoT issues node --version)

v20.9.0

Browser(s)

Chrome

Steps to reproduce the behavior

Reproduce steps:

  1. user just used a p-tree filter
  2. then lazyLoaded some additional nodes -
  3. onNodeClick do not emit event on newly added nodes click

Expected behavior

User can click on newly lazy loaded nodes even after applying filter on tree

@kievsash kievsash added the Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible label Feb 6, 2024
@kievsash
Copy link
Author

kievsash commented Feb 6, 2024

primeng-tree-filter-click-issue

@mehmetcetin01140 mehmetcetin01140 added Type: Bug Issue contains a bug related to a specific component. Something about the component is not working and removed Status: Needs Triage Issue will be reviewed by Core Team and a relevant label will be added as soon as possible labels Feb 8, 2024
@cetincakiroglu cetincakiroglu added this to the 17.7.0 milestone Feb 12, 2024
@cetincakiroglu cetincakiroglu self-assigned this Feb 13, 2024
@cetincakiroglu
Copy link
Contributor

Hi,
Could you please replace the codesandbox example by stackblitz. Codesandbox throws page not found error.

Screenshot 2024-02-14 at 17 39 18

@cetincakiroglu cetincakiroglu added the Resolution: Needs More Information More information about the issue is needed to find a correct solution label Feb 14, 2024
@cetincakiroglu cetincakiroglu modified the milestones: 17.7.0, 17.8.0 Feb 14, 2024
@kievsash
Copy link
Author

kievsash commented Feb 15, 2024

https://codesandbox.io/p/live/83658f35-d688-4541-8d7c-2fef898406d5

Hi @cetincakiroglu
I shared incorrect link, sorry
try this one plz: https://codesandbox.io/p/devbox/primeng-tree-lazy-demo-forked-hvm6ls
But checkout it to run locally, somehow it does start Angular app in a playground

@cetincakiroglu
Copy link
Contributor

Hi,

Your example does not start. Could you please share a stackblitz example instead of codesandbox. Codesandbox received an update and somehow some of the projects became broken so we've removed them from PrimeNG website. Moving it to the next milestone until the stackblitz example arrives.

Screenshot 2024-02-22 at 16 20 43

@cetincakiroglu cetincakiroglu modified the milestones: 17.8.0, 17.9.0 Feb 22, 2024
@kievsash
Copy link
Author

@cetincakiroglu
Copy link
Contributor

Hi,

Thanks for the example, it still doesn't work by the way. However, we'll try to reproduce it in our local and recheck the issue in 17.10.0

Screenshot 2024-02-29 at 18 00 14

@cetincakiroglu cetincakiroglu modified the milestones: 17.9.0, 17.10.0 Feb 29, 2024
@kievsash
Copy link
Author

Ping me if you have some reproduce questions, we can zoom if needed. kievsash@ukr.net

@cetincakiroglu cetincakiroglu added LTS-PORTABLE and removed Resolution: Needs More Information More information about the issue is needed to find a correct solution labels Mar 7, 2024
@cetincakiroglu
Copy link
Contributor

Hi,

Thank you very much for reporting the issue. The Tree component uses the key property to establish a context between child and parent nodes, and as you have seen in the source code, the key is used to search for the selected node. If the key is not found, the existing node is returned, causing the onNodeSelect operation to be called only once. When the component is clicked for the second time, without a key, it returns the same node and unselects the already selected node. This issue is related to the "files" value we use in the demo, where the "key" values are missing in the files received from the service, preventing the establishment of the parent-child relationship. Additionally, there is a mistake in the following line:

node = this.getNodeWithKey(<string>node.key, <TreeNode<any>[]>this.value) as TreeNode;

Here, instead of this.value, as you also mentioned, it should be this.filteredNodes. Making this change and fixing the demo makes the example work without any issues.

After today's release, you can simply copy-past and try the updated demo code above:

import { Component, OnInit, ChangeDetectorRef, ViewChild } from '@angular/core';
import { MessageService, TreeNode } from 'primeng/api';
import { NodeService } from '../../service/nodeservice';
import { Tree } from 'primeng/tree';

@Component({
  selector: 'tree-lazy-demo',
  template: `<div class="card flex justify-content-center">
  <p-tree #tree class="w-full md:w-30rem" [value]="nodes" [filter]="true" (onNodeExpand)="nodeExpand($event)" [loading]="loading" selectionMode="single" (onNodeSelect)="onNodeSelect($event)"></p-tree>
</div>`,
  providers: [MessageService],
})
export class TreeLazyDemo implements OnInit {
  loading: boolean = false;

  nodes!: TreeNode[];

  @ViewChild('tree') tree: Tree;

  constructor(private cd: ChangeDetectorRef) {}

  ngOnInit() {
    this.loading = true;
    setTimeout(() => {
      this.nodes = this.initiateNodes();
      this.loading = false;
      this.cd.markForCheck();
    }, 2000);
  }

  initiateNodes(): TreeNode[] {
    return [
      {
        key: '0',
        label: 'Node 0',
        leaf: false,
      },
      {
        key: '1',
        label: 'Node 1',
        leaf: false,
      },
      {
        key: '2',
        label: 'Node 2',
        leaf: false,
      },
    ];
  }

  nodeExpand(event: any) {
    console.log('onNodeExpand', event);
    if (!event.node.children) {
      this.loading = true;
      setTimeout(() => {
        event.node.children = [];
        for (let i = 0; i < 3; i++) {
          event.node.children.push({
            key: event.node.key + '-' + i,
            label: 'Node ' + event.node.key + '-' + i,
            leaf: false,
          });
        }
        this.loading = false;
        this.cd.markForCheck();
      }, 500);
    }
  }

  onNodeSelect(event) {
    console.log('onNodeSelect', event);
  }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LTS-PORTABLE Type: Bug Issue contains a bug related to a specific component. Something about the component is not working
Projects
None yet
Development

No branches or pull requests

3 participants