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

fix: prevent material tabBarItem from re-rendering when using material-top-tabs #11385

Closed

Conversation

okwasniewski
Copy link
Contributor

@okwasniewski okwasniewski commented May 26, 2023

Motivation

This issue addresses performance problems when using material-top-tabs and having many tabs. The problem was with passed callbacks which were causing all TabBarItems to re-render on index change. It should address this issue: #11047

Before:
CleanShot 2023-05-26 at 12 12 12@2x

After:
CleanShot 2023-05-26 at 12 10 47@2x

Test plan

Launch Material Top Tabs example and check that only necessary tab items are re-rendering.

@netlify
Copy link

netlify bot commented May 26, 2023

Deploy Preview for react-navigation-example ready!

Name Link
🔨 Latest commit fc53d5f
🔍 Latest deploy log https://app.netlify.com/sites/react-navigation-example/deploys/6470a9f08a04b500091dafa1
😎 Deploy Preview https://deploy-preview-11385--react-navigation-example.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02 🎉

Comparison is base (1613461) 75.43% compared to head (6137405) 75.45%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11385      +/-   ##
==========================================
+ Coverage   75.43%   75.45%   +0.02%     
==========================================
  Files         189      189              
  Lines        5712     5717       +5     
  Branches     2254     2253       -1     
==========================================
+ Hits         4309     4314       +5     
  Misses       1353     1353              
  Partials       50       50              
Impacted Files Coverage Δ
packages/react-native-tab-view/src/TabBarItem.tsx 75.60% <100.00%> (+1.58%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@okwasniewski okwasniewski force-pushed the @okwasniewski/material-top-tabs/re-renders branch from 6137405 to d5b3e41 Compare May 26, 2023 12:38
@okwasniewski okwasniewski force-pushed the @okwasniewski/material-top-tabs/re-renders branch from d5b3e41 to fc53d5f Compare May 26, 2023 12:45
@andonivianez
Copy link

When is this going to be published? Is urgent, as material-top-tabs are laggy when you touch the tabs.

@okwasniewski
Copy link
Contributor Author

When is this going to be published? Is urgent, as material-top-tabs are laggy when you touch the tabs.

Unfortunately, this PR can't be merged as-is because the additional memoization breaks some stuff with rendering. I'm going to work today on some performance boosts for material-top-tabs however, there will be RFC with breaking API changes proposed to make this faster and released as a new major version of tab-view.

@okwasniewski
Copy link
Contributor Author

Closing this PR all issues with material-top-tabs performance should be addressed after this RFC: react-navigation/rfcs#88

@github-actions
Copy link

Hey! This issue is closed and isn't watched by the core team. You are welcome to discuss the issue with others in this thread, but if you think this issue is still valid and needs to be tracked, please open a new issue with a repro.

@satya164 satya164 deleted the @okwasniewski/material-top-tabs/re-renders branch June 17, 2023 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants