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

TabView: Dynamically created TabPanels and onTabClose closes more than one Tab #2842

Closed
maximilianweidenauer opened this issue May 5, 2022 · 8 comments · Fixed by #5239
Closed
Labels
Type: Bug Issue contains a defect related to a specific component.
Milestone

Comments

@maximilianweidenauer
Copy link

Describe the bug

I have a TabView with dynamically created TabPanels, some are closable and some are disabled. When closing one of them, an additional TabPanel gets closed or isn't visible anymore. In my example there are 5 Tabs when closing the third, the forth won't be displayed aswell. (In my demo, the activeIndex stays 0 because in my app it shouldn't switch on close, when it isn't the currently selected one)

Reproducer

https://codesandbox.io/s/misty-worker-1lu9yy?file=/src/demo/TabViewDemo.js

PrimeReact version

7.2.1 (tested with 8.0.1 too)

React version

17.x

Language

TypeScript

Build / Runtime

Create React App (CRA)

Browser(s)

Chrome

Steps to reproduce the behavior

  1. Close the third Tab
  2. Forth Tab will disappear aswell

Expected behavior

When I close the third Tab, only the third Tab gets removed.

@maximilianweidenauer maximilianweidenauer added the Type: Bug Issue contains a defect related to a specific component. label May 5, 2022
@melloware melloware added the Status: Pending Review Issue or pull request is being reviewed by Core Team label May 5, 2022
@melloware
Copy link
Member

Excellent reproducer. Definitely a bug

@melloware melloware added confirmed and removed Status: Pending Review Issue or pull request is being reviewed by Core Team labels May 18, 2022
@melloware melloware linked a pull request Sep 8, 2022 that will close this issue
@melloware melloware added this to the 8.6.0 milestone Sep 8, 2022
@mertsincan mertsincan modified the milestones: 8.6.0, 8.6.1, 8.6.2 Sep 22, 2022
@mertsincan mertsincan modified the milestones: 8.7.0, 8.7.1, 8.7.2 Oct 25, 2022
@mertsincan mertsincan modified the milestones: 8.7.2, 8.7.3 Nov 10, 2022
@mertsincan mertsincan modified the milestones: 8.7.3, 8.7.4 Dec 5, 2022
@mertsincan mertsincan modified the milestones: 8.7.4, 10.0.0 Jan 26, 2023
@habubey habubey modified the milestones: 10.0.0, 9.2.2 Mar 7, 2023
@ulasturann
Copy link
Contributor

The ".splice()" method refers the same array, so it might be more suitable to use the "filter" method as shown below:

                        onTabClose={(e) => {
                            const newTabArray = [...components];

                            newTabArray.filter((tab, i) => i !== e.index);
                            setComponents(newTabArray);
                        }}

@maximilianweidenauer
Copy link
Author

My reproducer didn't exactly do, what my react project does... In my project I'm sending a request to a server and the server tells me to remove this component. The server can remove a lot of components that's why I have an extra hook for that.
But I tried to kind of simulate what it would do and I have changed this in the reproducer:

          onTabClose={(e) => {
            const newTabArray = [];
            components.forEach((comp) => {
              if (comp.id !== e.index) {
                newTabArray.push(comp);
              }
            });
            setComponents(newTabArray);
          }}

Then I'm experiencing the same issues as before. Btw. in my app I'm not checking with comp.id !== e.index this is just for convenience in the reproducer, it would look more like this in the custom hook I mentioned.

            /** New Components when component changes */
            const newComponents = buildComponents();
            /** Contains the components */
            const cl = new Array<ReactElement>();
            newComponents.forEach(nc => {
                let alreadyAdded = false
                /** Checks if the new component is already added in the current components if yes add the old component else the new one */
                components.forEach(oc => {
                    const objectKeys = Object.keys(oc.props).filter(key => key !== "onLoadCallback");
                    if(nc.props.id === oc.props.id && !context.contentStore.customComponents.has(nc.props.name) && _.isEqual(getExtractedObject(oc.props, objectKeys), getExtractedObject(nc.props, objectKeys))) {
                        alreadyAdded = true
                        cl.push(oc);
                    }
                });
                if(!alreadyAdded && !context.contentStore.removedCustomComponents.has(nc.props.name)) {
                    cl.push(nc);
                }
            });
            setComponents(cl);

@maximilianweidenauer
Copy link
Author

buildComponents() returns the children of a component as ReactElements thats how the TabView would know that a Tab has been deleted, because buildComponents would return one element less. In case that helps to clarify

@maximilianweidenauer
Copy link
Author

And newTabArray.filter((tab, i) => i !== e.index); wouldn't do anything right?
Wouldn't it have to be newTabArray = newTabArray.filter((tab, i) => i !== e.index); to actually change the array? Because when I was logging after the filter, the Array still had a length of 5.

@maximilianweidenauer
Copy link
Author

The issue maybe is, that PrimeReact is closing a Tab when clicking the 'X' and I'm also removing a Tab. So 2 Tabs are removed? Maybe there could be a mode for the TabView where PrimeReact doesn't close Tabs and lets the User handle the closing?

@diogoko
Copy link

diogoko commented Mar 17, 2023

@maximilianweidenauer there's this PR that adds closeMode="manual" for this exact use case.

@melloware
Copy link
Member

This is fixed by: #5229

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issue contains a defect related to a specific component.
Projects
None yet
6 participants