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(android): fix Material3 BottomNavigation height #14110

Merged
merged 5 commits into from
Nov 9, 2024

Conversation

m1ga
Copy link
Contributor

@m1ga m1ga commented Sep 10, 2024

New PR:

I've only add the note in the docs as I've started a rewrite of the BottomNavigation in another PR.

Old PR (just as a reference):

SO issue in native land: https://stackoverflow.com/questions/72062902/how-do-i-get-the-bottom-navigation-bar-height-for-material-design-3 and some screenshots.

Currently we use design_bottom_navigation_height for the height of the bottom navigation (56dp). It looks like Material 3 is using m3_bottom_nav_min_height (80dp).

We need to check if the app is using a Material3 theme and then use the new dimens value.

Test

var win1 = Ti.UI.createWindow({});
var tf = Ti.UI.createTextField({ bottom: 0, width: Ti.UI.FILL, borderWidth: 1, borderColor: "yellow"})
var win2 = Ti.UI.createWindow({});
win1.add(tf);

var tab1 = Ti.UI.createTab({ window: win1, title: 'Blue' }),
	tab2 = Ti.UI.createTab({ window: win2, title: 'Red'}),
	tabGroup = Ti.UI.createTabGroup({ tabs: [tab1, tab2],
		theme: "Theme.Titanium.Material3.DayNight",                              // <- remove for Material 2
		style: Titanium.UI.Android.TABS_STYLE_BOTTOM_NAVIGATION
	});
tabGroup.open();

Yellow text field should be fully visible in the tab.

Questions:
Can we use activity.getTheme().toString().indexOf("Material3") to check for Material3 theme?

Workaround
If you use M3 in your app you can add /app/platform/android/res/values/dimens.xml with

<?xml version="1.0" encoding="utf-8"?>
<resources>
    <dimen name="design_bottom_navigation_height">80dp</dimen>
</resources>

to override the current value.

@m1ga m1ga marked this pull request as ready for review October 5, 2024 13:52
@m1ga
Copy link
Contributor Author

m1ga commented Oct 5, 2024

This will just add a note to the docs now. As many people still use the old M2 themes and I've started a rewrite of the BottomNavigation in a different PR it should be enough for now. If you use a M3 theme you have to add the XML file with the height yourself

@hansemannn
Copy link
Collaborator

It can only be seen as a workaround, as we definitely need the "clean" solution from the other PR

Copy link
Collaborator

@hansemannn hansemannn left a comment

Choose a reason for hiding this comment

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

Using this one until the new bottom nav is finished.

@hansemannn hansemannn merged commit d83d64e into master Nov 9, 2024
7 checks passed
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