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: change of width of tabBarIndicator due to change of leftPadding #11713

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

g4ps
Copy link

@g4ps g4ps commented Nov 21, 2023

Motivation

The issue manifests itself only on the android phones (as far as I know).

When we try to add paddingLeft and width: 'auto' to contentConteinerStyle in TabBar, we get a
certain problem: our TabBarIndicator's width goes completely out of whack. This problem is due to
a more general problem in react-native, which can't properly process large values of scaleX
transforms when position of the block is either 'relative' or 'absolute' and 'left' is defined
on android. There are two main ways to solve this problem: the first one is to wail
'till the folks in react-native community solve the underlying issue, and the second
(and way more attractive in my personal opinion) solution is to make width of the tabBarElement
on android larger so that we don't have to deal with scaleX's in triple digits. This results
with a tad bit more complicated maths in TabBarIndicator, particularly when it comes to calculating
translations after the scaling, but notihng too complicated.

Test plan

  • Get yourself an android phone
  • go to https://github.com/g4ps/testApp, download and install (yarn && yarn run start) the app
  • change the values of paddingLeft in contentContainerStyle in tabBarStyles (values between 20 and
    28 should do)
  • you should see something like this, where the width of the TabBarIndicator is gonna be completely
    out of whack:
document_5397965453672856642.mp4
  • change "react-native-tab-view": "^4.0.0-alpha.2" to "react-native-tab-view": "g4ps/react-navigation.git#react-native-tab-view-v4.0.0-alpha.2-gitpkg" (which was build with this PR) in package.json
  • reinstall the app
  • change the values of paddingLeft to whatever value your heart desires, and everything's
    going to be completely within the acceptable levels of whack

Copy link

Hey @g4ps! Thanks for opening your first pull request in this repo. If you haven't already, make sure to read our contribution guidelines.

@codecov-commenter
Copy link

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (8e7ac4f) 75.96% compared to head (f3d1639) 75.88%.

Files Patch % Lines
...ages/react-native-tab-view/src/TabBarIndicator.tsx 25.00% 9 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #11713      +/-   ##
==========================================
- Coverage   75.96%   75.88%   -0.09%     
==========================================
  Files         207      207              
  Lines        5942     5951       +9     
  Branches     2302     2307       +5     
==========================================
+ Hits         4514     4516       +2     
- Misses       1378     1385       +7     
  Partials       50       50              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

netlify bot commented Nov 21, 2023

Deploy Preview for react-navigation-example ready!

Name Link
🔨 Latest commit f3d1639
🔍 Latest deploy log https://app.netlify.com/sites/react-navigation-example/deploys/655d0e8c92f0190008de6849
😎 Deploy Preview https://deploy-preview-11713--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 configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants