React Tabbar, update page when not passing explicit prop in. #2508
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue #2335 is happening due to the Tabbar components internal page cache being incorrectly updated when the Page component returned from the renderTabs function changes.
In Tabbar's render method:
The first render of the component will populate tabPages with every tabs content. Subsequent renders will only update the current tab's content. A page's content fails to update when the 'index' prop is out of sync with the actual index that needs to be update. This can occur for two reasons:
So what happens in case 1? The default for the index prop is 0. As such, changes to the first tab's content will be made, all other tabs will be ignored.
In case 2, the 'index' prop can be out of sync with the ons-tabbar's active tab index since it can change by means other than just updating the Tabbar's 'index' prop.
In Tabbar's componentWillReceiveProps method:
How do we solve this problem? The easiest solution would be to regenerate the entire tabPages cache on every render. This has the downside of removing the entire optimization and results in unnecessary reconciliation within react. A better solution is keeping the current pattern, but ensuring that the correct tab is updated.
Conceptually, what we want is this:
Notice how the responsibility of deciding which tab's content gets updated has changed from the passed in 'index' prop to the ons-tabbar's current active tab.
There are 2 issues with this solution though:
The first point seems like a non-issue, since renderTabs' output can't change without a rerender happening. It is an issue though. We can't be sure that renderTab's output hadn't changed during a prior rendering, which was thrown out by not updating tabPages to prevent unnecessary reconciliation.
The second point results from the possibility of setting ons-tabbar's active tab being asynchronous. This appears to be rare, but happens when:
A. A tab's page is not loaded.
B. platform.isUIWebView() is true.
To ensure that react rerenders the component and uses the correct tab index in response to the active tab changing, this patch tracks the active tab within Tabbar's state, which gets updated during the onPostChange event or in response to the 'index' prop changing. I have tested it with the example in issue #2335 to confirm it works, and I am unaware of any side-effects or bugs.