primereact icon indicating copy to clipboard operation
primereact copied to clipboard

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

Open maximilianweidenauer opened this issue 3 years ago • 1 comments

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 avatar May 05 '22 07:05 maximilianweidenauer

Excellent reproducer. Definitely a bug

melloware avatar May 05 '22 15:05 melloware

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);
                        }}

ulasturann avatar Mar 13 '23 13:03 ulasturann

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 avatar Mar 13 '23 14:03 maximilianweidenauer

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 avatar Mar 13 '23 14:03 maximilianweidenauer

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 avatar Mar 13 '23 14:03 maximilianweidenauer

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?

maximilianweidenauer avatar Mar 13 '23 15:03 maximilianweidenauer

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

diogoko avatar Mar 17 '23 21:03 diogoko

This is fixed by: https://github.com/primefaces/primereact/issues/5229

melloware avatar Nov 06 '23 18:11 melloware