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: fix tabLabelStyle not applying on android #251

Closed
wants to merge 1 commit into from

Conversation

owinter86
Copy link
Contributor

PR Description

tabLabelStyle is not applied on android currently in the latest release, this fixes this issue so that custom font and font size changes will correctly apply.

How to test?

Change fontSize in the tabLabelStyle within the NativeBottomTabs route, pre this commit it did not do anything.

@owinter86
Copy link
Contributor Author

owinter86 commented Jan 21, 2025

I also notice that the current react font manager instance only allows for font weight changes from bold or normal, other weights will default back to normal.

val typeface = ReactFontManager.getInstance().getTypeface(
fontFamily ?: "",
Utils.getTypefaceStyle(fontWeight),
context.assets
)

however I also see that its possible to utilise the range of fontWeights within the ReactFontManager class

public Typeface getTypeface(String fontFamilyName, int weight, boolean italic, AssetManager assetManager) {
  return this.getTypeface(fontFamilyName, new TypefaceStyle(weight, italic), assetManager);
}

I tested this locally and was able to have various font weights ranging from 100 to 900 (depending on the font support)

val typeface = ReactFontManager.getInstance().getTypeface(
  fontFamily ?: "",
  fontWeight ?: 400,
  false,
  context.assets
)

not sure if there were any reason behind the restricted font weight selections, and think its probably outside the scope of this quick fix. But maybe worth adding to this, or a different PR.

@@ -370,7 +370,7 @@ class ReactBottomNavigationView(context: ReactContext) : LinearLayout(context) {

private fun updateTextAppearance() {
if (fontSize != null || fontFamily != null || fontWeight != null) {
val menuView = getChildAt(0) as? ViewGroup ?: return
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for creating this PR, looks like this change is already included in #250

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see, looks like it snuck in on my previous PR and not isolated here.

@okwasniewski
Copy link
Collaborator

I also notice that the current react font manager instance only allows for font weight changes from bold or normal, other weights will default back to normal.

val typeface = ReactFontManager.getInstance().getTypeface(
fontFamily ?: "",
Utils.getTypefaceStyle(fontWeight),
context.assets
)

however I also see that its possible to utilise the range of fontWeights within the ReactFontManager class

public Typeface getTypeface(String fontFamilyName, int weight, boolean italic, AssetManager assetManager) {
  return this.getTypeface(fontFamilyName, new TypefaceStyle(weight, italic), assetManager);
}

I tested this locally and was able to have various font weights ranging from 100 to 900 (depending on the font support)

val typeface = ReactFontManager.getInstance().getTypeface(
  fontFamily ?: "",
  fontWeight ?: 400,
  false,
  context.assets
)

not sure if there were any reason behind the restricted font weight selections, and think its probably outside the scope of this quick fix. But maybe worth adding to this, or a different PR.

You are welcome to create a PR for this support, It seemed off to me when I implemented this and forgot to get back to this later 😅

@owinter86
Copy link
Contributor Author

@okwasniewski ok I will update this to include the additional font weight supports.

@owinter86 owinter86 closed this Jan 24, 2025
@owinter86 owinter86 deleted the tabLabelStyle branch January 24, 2025 07:00
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.

2 participants