Closed RSantosGIS closed 1 month ago
@RSantosGIS Could you put together a code sample for us to troubleshoot using a CodeSandbox Calcite and React template?
@geospatialem Unfortunately, I cannot right now. We are in the home stretch to code freeze for Studio and the best I can do is write up Calcite issues when I find them in the course of our work. I'd be happy to hop on a call to walk someone through it if it's helpful.
@geospatialem This is become a bigger issue for us because there appears to be other bugged behavior associated with closing a tab when it is part of a dynamically generated tab list from state. I made a sandbox that shows a simple case of things breaking here, and will add it to the issue: https://codesandbox.io/s/suspicious-elion-hk8tgf?file=/src/App.js https://codesandbox.io/s/funny-sound-6vn45x?file=/src/App.js
I'm also now observing behavior where if I close a tab to the left of the active tab, the user cannot navigate to the tab immediately to the right of the active tab without first clicking on some other tab. There appears to be some kind of hidden index tracking inside tabs that is getting messed up. So for example, if your active tab is in slot 4 and you close a tab in slot 2, the active tab moves into slot 3 and you cannot now open the tab in slot 4.
Unfortunately, I cannot produce a sandbox for this beyond the one I already made because the first error I reported already crashes the sample. But I believe it is almost certainly also the same root cause - tab closing doesn't know how to properly handle the tab also being removed from the DOM
I found a couple of issues in the sandbox that where making the behavior strange. Here are my corrections that work https://codesandbox.io/s/upbeat-fast-yfwn23
.splice
does NOT return the modified array but the deleted items, so even if everything had re-rendered as expected, the onTabClose
function was setting the new tab config state to the deleted item and not the updated array.key
parameter to the <CalciteTabs>
component that is unique to the configured tabs. In this example, I simply joined the tab title strings together. The reason you need to do this is that you are modifying the internal parts of a web component that the web component cares about and tracks. Therefore you need to instruct react to fully re-render the web component itself. Without the key on the parent web component element, you are getting the web component internal state and the dom state out of sync and then things don't work anymore. I've had this issue with other web components that keep track of their children via slot
attributes. Changing a child does not reset the parent's state of trigger a re-render of the parent directly.Hey @mpriour , thanks for taking a look at this. Summarizing our conversation on teams and adding a little more:
I still feel like this is something you should take a look at. I would expect when closing the tab that no part of the CalciteTabs component would still rely on the tab existing internally. The fact that if you close a tab and then it disappears from the DOM the breaks the navigation for the tab immediately to the right of the active tab still feels like a bug, because a full re-render of the component to resync everything causes you to lose capabilities.
Updated my sample with Matt's suggested change, and it still demonstrates both problems. Repro steps:
@RSantosGIS Could you update the repro case or the steps above to make sure they're in sync? The latest sample looks as follows:
Just making sure we're reproducing accurately.
@jcfranco ugh, no it's not. I will do it again and update
@jcfranco finally got around to updating this, sorry I've been slammed with Studio's end of iteration. https://codesandbox.io/s/funny-sound-6vn45x?file=/src/App.js
Please let me know if the example or anything about what we are trying to achieve is unclear
@mpriour suggested that changing CalciteTab before CalciteTabTitle resolves the null access error, and I confirmed that indeed it does. However, it does not resolve the navigation problem: navigate to tab 3 of 4 close tab 2 attempting to navigate to the tab that now occupies slot 3 (since they all moved one to the left) does not work
Still, it might be worth changing the docs in the interim to at least recommend the order change.
Updated the repro case to remove the extraneous components in the sandbox and updated to 1.5.1
. Triaging will occur in the next few weeks.
This issue isn't specific to React: https://codepen.io/eriklharper/pen/JjqXKxO
Installed and assigned for verification.
Verified with https://codepen.io/eriklharper/pen/JjqXKxO in 2.11.0-next.21
. 🎉
Actual Behavior
edit: please see conversation for description of additional issue with navigation between tabs also breaking, related to this same root cause.
I started using the closable tabs recently and overall I think it's great work. I observed a few issues during my time getting things setup (this is in React):
Expected Behavior
Reproduction Sample
ArcGIS Knowledge Studio https://codesandbox.io/s/summer-mountain-y3jcf3?file=/src/App.js
Reproduction Steps
To reproduce in the sandbox updated to 1.5.1:
To reproduce in Studio:
For the events:
For the null access error:
Reproduction Version
1.4.2
Relevant Info
No response
Regression?
No response
Priority impact
p3 - want for upcoming milestone
Impact
I can work around the lack of events firing using other handlers, and we are currently just suppressing the error from the null access. Because of the way the code is written where the error is occurring inside a series of async functions that are called for each tab index, the fact that the call fails with an uncontrolled exception for the tab that disappeared from the DOM doesn't actually break anything for us... I think. Obviously it makes me nervous 😄
Esri team
ArcGIS Knowledge