[Android] Implement fixed mode for Bottom Navigation Bar Android #4003

Viridovics · 2018-10-05T01:44:05Z

Description of Change

Add platform specific api:

On<Android>().SetBottomToolbarShiftMode(ToolbarShiftMode.Fixed, ToolbarShiftMode.Fixed);

Issues Resolved

related to [Enhancement] Implement fixed mode for Bottom Navigation Bar Android #3083

API Changes

Added:

ToolbarShiftMode GetToolbarShiftMode(BindableObject element)
ToolbarShiftMode GetToolbarShiftMode(this IPlatformElementConfiguration<Android, FormsElement> config)
ToolbarShiftMode GetToolbarItemShiftMode(BindableObject element)
ToolbarShiftMode GetToolbarItemShiftMode(this IPlatformElementConfiguration<Android, FormsElement> config)
void SetBottomToolbarShiftMode(BindableObject element, ToolbarShiftMode shiftMode, ToolbarShiftMode itemShiftMode)
IPlatformElementConfiguration<Android, FormsElement> SetBottomToolbarShiftMode(this IPlatformElementConfiguration<Android, FormsElement> config, ToolbarShiftMode toolbarShiftMode, ToolbarShiftMode toolbarItemShiftMode)

Platforms Affected

Android

Behavioral/Visual Changes

Before/After Screenshots

Not applicable

Testing Procedure

Ui test BottomNavigationShiftModes

PR Checklist

Has automated tests
Rebased on top of the target branch at time of PR
Changes adhere to coding standard

Viridovics · 2018-10-05T01:49:43Z

I'm not sure about this try/catch:

                try
                {
                 ...
		}
		catch(Exception e)
		{
			System.Diagnostics.Debug.WriteLine($"Unable to set shift mode: {e}");
		}

AmrAlSayed0 · 2018-10-06T11:56:30Z

According to this StackOverflow question, using reflection will cause problems when ProGaurd is enabled. It will strip the internal non-public fields of the BottomNavigationView and this fix will not work. If this gets merged, this should be atleast documented somewhere.

Viridovics · 2018-10-06T15:13:22Z

Thank you @AmrAlSayed0 !
I think that ProGuard problem can be resolved in #2709

rmarinho · 2018-10-18T16:08:16Z

Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs

+				if (menuView == null)
+					return;
+
+				var shiftMode = menuView.Class.GetDeclaredField("mShiftingMode");


What if this was obfuscated by Proguard? we might need to add some rules to it.

One thing to note about this is that with API 28 google is grey listing reflection calls

https://developer.android.com/about/versions/pie/restrictions-non-sdk-interfaces

We have an issue to look at this
#4118

It looks like Version 28 of the support libraries is going to support setting this so we'll need to determine if we want to add a grey listed api like this until we can upgrade Xamarin Forms to the newer support libraries

LabelVisibilityMode has different behaviour than api function SetBottomToolbarShiftMode.
And problems that related with reflection (grey list, Proguard, etc) are pretty unpleasant.

If we want to wait for new api, we will lose this functionality only for api 26,27 (if I correctly understand how Xamarin forms and support libraries work).

I think we should wait until Xamarin forms will be upgraded to the newer support libraries.

P.S. And after that I will correct review remarks and suggestions.

rmarinho

I don't think we should use reflection. I have look around the web and seems other's on "Java" work do the same. But we will need to try to find another workaround since Proguard or other obfuscation/linker tool can strip that property.

rmarinho · 2018-10-18T16:08:37Z

Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs

+					return;
+
+				var shiftMode = menuView.Class.GetDeclaredField("mShiftingMode");
+				shiftMode.Accessible = true;


I do we set true, and then False?

It's just the whacky way you have to do it
https://stackoverflow.com/questions/40176244/how-to-disable-bottomnavigationview-shift-mode

The other thing to note is that android looks to be adding an API for doing all of this in the upcoming support libraries

<android.support.design.widget.BottomNavigationView app:labelVisibilityMode="labeled" />

rmarinho · 2018-10-18T16:08:50Z

Xamarin.Forms.Platform.Android/AppCompat/TabbedPageRenderer.cs

+				shiftMode.Accessible = true;
+				shiftMode.SetBoolean(menuView, IsEnableShiftMode);
+				shiftMode.Accessible = false;
+				shiftMode.Dispose();


Do we really need to call Dispose?

rmarinho · 2018-10-18T16:12:57Z

Xamarin.Forms.Core/PlatformConfiguration/AndroidSpecific/TabbedPage.cs

@@ -150,6 +150,48 @@ public static int GetMaxItemCount(this IPlatformElementConfiguration<Android, Fo
 			return GetMaxItemCount(config.Element);
 		}

+		public static readonly BindableProperty ToolbarShiftModeProperty =


I wonder if we can have the same property for both, the TabbedPage and the Items, instead of having 2 different properties with different naming.

Maybe add ShiftMode to Page ?

PureWeen · 2018-10-30T16:21:56Z

I tested this with v28 of the support libraries and the new setting. It looks like that setting will work for what we need. It's interesting because it looks like the default LabelVisibilityMode with support 28 is the non shifting one. Which is annoying that the behavior is going to just change when updating to v28.

LabelVisibilityMode has 4 different available values and we will probably surface this change by surfacing an API for that ENUM into the platform specifics (I'll modify the original ticket)

I've included a screen shot of how it looks using the new setting in v28

PureWeen · 2018-12-19T02:42:57Z

FYI @Viridovics we recently added this

https://github.com/xamarin/Xamarin.Forms/blob/78385f9fc1fc56dc88bd98e73bf9c8f2f2d0a90a/Xamarin.Forms.Platform.Android/Renderers/BottomNavigationViewUtils.cs

If you feel like updating this PR to use that then we can look at getting this reopened and merged

Add shift mode for bottom navigation

e59dff6

Viridovics changed the title ~~Implement fixed mode for Bottom Navigation Bar Android~~ [Android] Implement fixed mode for Bottom Navigation Bar Android Oct 5, 2018

samhouts added p/Android t/enhancement ➕ labels Oct 5, 2018

samhouts assigned jassmith and PureWeen Oct 5, 2018

samhouts requested review from jassmith and PureWeen October 5, 2018 17:56

xamarin-release-manager added the API-change Heads-up to reviewers that this PR may contain an API change label Oct 5, 2018

PureWeen requested review from rmarinho and removed request for jassmith October 16, 2018 17:46

PureWeen assigned rmarinho and unassigned jassmith Oct 16, 2018

rmarinho reviewed Oct 18, 2018

View reviewed changes

rmarinho suggested changes Oct 18, 2018

View reviewed changes

PureWeen added the DO-NOT-MERGE-!!! 🛑 This is in progress and needs to be updated before it can be merged. label Oct 20, 2018

Merge branch 'master' into issue-3083

a07dcea

PureWeen closed this Oct 30, 2018

PureWeen mentioned this pull request Feb 7, 2019

[Enhancement] Implement fixed mode for Bottom Navigation Bar Android #3083

Closed

samhouts added the hacktoberfest 🍻 label Oct 1, 2019

samhouts added the in-progress This issue has an associated pull request that may resolve it! label Oct 2, 2019

samhouts added the proposal-open label Feb 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Android] Implement fixed mode for Bottom Navigation Bar Android #4003

[Android] Implement fixed mode for Bottom Navigation Bar Android #4003

Viridovics commented Oct 5, 2018 •

edited by samhouts

Loading

Viridovics commented Oct 5, 2018

AmrAlSayed0 commented Oct 6, 2018

Viridovics commented Oct 6, 2018

rmarinho Oct 18, 2018

PureWeen Oct 18, 2018

Viridovics Oct 21, 2018

rmarinho left a comment

rmarinho Oct 18, 2018

PureWeen Oct 18, 2018 •

edited

Loading

rmarinho Oct 18, 2018

rmarinho Oct 18, 2018

PureWeen commented Oct 30, 2018

PureWeen commented Dec 19, 2018 •

edited

Loading

[Android] Implement fixed mode for Bottom Navigation Bar Android #4003

[Android] Implement fixed mode for Bottom Navigation Bar Android #4003

Conversation

Viridovics commented Oct 5, 2018 • edited by samhouts Loading

Description of Change

Issues Resolved

API Changes

Platforms Affected

Behavioral/Visual Changes

Before/After Screenshots

Testing Procedure

PR Checklist

Viridovics commented Oct 5, 2018

AmrAlSayed0 commented Oct 6, 2018

Viridovics commented Oct 6, 2018

rmarinho Oct 18, 2018

Choose a reason for hiding this comment

PureWeen Oct 18, 2018

Choose a reason for hiding this comment

Viridovics Oct 21, 2018

Choose a reason for hiding this comment

rmarinho left a comment

Choose a reason for hiding this comment

rmarinho Oct 18, 2018

Choose a reason for hiding this comment

PureWeen Oct 18, 2018 • edited Loading

Choose a reason for hiding this comment

rmarinho Oct 18, 2018

Choose a reason for hiding this comment

rmarinho Oct 18, 2018

Choose a reason for hiding this comment

PureWeen commented Oct 30, 2018

PureWeen commented Dec 19, 2018 • edited Loading

Viridovics commented Oct 5, 2018 •

edited by samhouts

Loading

PureWeen Oct 18, 2018 •

edited

Loading

PureWeen commented Dec 19, 2018 •

edited

Loading