Skip to content
This repository was archived by the owner on May 1, 2024. It is now read-only.

[REVERTED] [Android] Fix BottomNavigationItemView issue with MasterDetailPage #9187

Merged
merged 3 commits into from
Feb 18, 2020

Conversation

kvpt
Copy link
Contributor

@kvpt kvpt commented Jan 13, 2020

Description of Change

On certains conditions MasterDetailPage can be in conflict with BottomNavigationView.
Like this :
Screenshot_1578940463

The issue disappear if the BottomNavigationItem id are correctly set.
The BottomNavigationItem id was 0, 1, 2 ... which seem to cause issue with the ViewPager because these id are already used is others views in Android.
ViewId

I tried to make an UI test case based on the reproduction in #9143, and you know what, impossible to reproduce in the UI test app. The problem seem to only occur during the app start.

Issues Resolved

API Changes

None

Platforms Affected

  • Android

Behavioral/Visual Changes

None

Before/After Screenshots

Not applicable

Testing Procedure

Use reproduction in #9143

PR Checklist

  • Targets the correct branch
  • Tests are passing (or failures are unrelated)

@samhouts samhouts added p/Android 4.3.0 regression on 4.3.0 i/regression t/bug 🐛 retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. labels Jan 13, 2020
@jsuarezruiz
Copy link
Contributor

@kvpt The fix makes sense to me. However, it would be great if we could add a repro sample to validate it in Core Gallery. I downloaded the repro sample from #9143 but there must be an error since it is an App using the empty project template.

The structure that causes the error is:

MainPage = MasterDetailPage:

Master: a Navigation page with a single Content page
Detail: a Tabbed page with 5 tabs:
--- each tab is a Navigation page with a single Content page in it
Toolbar placement is set to Bottom for Android.

Right?.

@kvpt
Copy link
Contributor Author

kvpt commented Jan 16, 2020

@jsuarezruiz The code of the reproduction is in the App.xaml.cs.
I tried to move it in a TestCase, but the issue doesn't appear there.
I don't really understand why it cause a problem with this reproduction and not in the others cases.

@samhouts samhouts removed the retarget-branch-required PR or associated issues target a milestone. Please target this PR to the matching branch. label Jan 16, 2020
@jsuarezruiz
Copy link
Contributor

@kvpt Could you attach the repro sample again?. I would like to add a test to validate the issue. I have looked at #9143 but the App.xaml.cs in the attached repro sample is empty.

@kvpt
Copy link
Contributor Author

kvpt commented Jan 28, 2020

@jsuarezruiz
It's really strange, I re-checked it and the App.xaml.cs code is present for me.
I copied it in a gist : https://gist.github.com/kvpt/0e8b099773e3c89e933c09518ef013cf

@jsuarezruiz
Copy link
Contributor

I added a repro sample, although I can't reproduce the issue in Core Gallery.

@samhouts samhouts added the approved Has two approvals, no pending reviews, and no changes requested label Feb 18, 2020
@samhouts samhouts merged commit a796678 into xamarin:master Feb 18, 2020
@PureWeen
Copy link
Contributor

It seems like this doesn't reproduce because the ID overlap happens here

Once you're in the context of the gallery the generated ids aren't at a value that will overlap any menu items

@samhouts samhouts changed the title [Android] Fix BottomNavigationItemView issue with MasterDetailPage [REVERTED] [Android] Fix BottomNavigationItemView issue with MasterDetailPage Feb 27, 2020
@samhouts samhouts added this to the 4.6.0 milestone Feb 28, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4.3.0 regression on 4.3.0 approved Has two approvals, no pending reviews, and no changes requested i/regression p/Android reverted t/bug 🐛
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants