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

[Bug] navPage.SetHideNavigationBarSeparator(true) no longer works. #9456

Closed
Tommigun1980 opened this issue Feb 7, 2020 · 20 comments · Fixed by #11165
Closed

[Bug] navPage.SetHideNavigationBarSeparator(true) no longer works. #9456

Tommigun1980 opened this issue Feb 7, 2020 · 20 comments · Fixed by #11165

Comments

@Tommigun1980
Copy link

Tommigun1980 commented Feb 7, 2020

Description

After updating to Xamarin.Forms 4.5.0.266-pre3 from the previous preview package, navPage.On().SetHideNavigationBarSeparator(true) no longer does anything. In other words, it seems to be impossible to hide the navigation bar separator in 4.5.0 pre-3.

Furthermore, my custom renderer that used to fix the navigation bar issues on iOS (that should become redundant after the fixes) had a ViewWillAppear override, that set some NavigationBar properties. After the update, NavigationBar.TitleTextAttributes and NavigationBar.LargeTitleTextAttributes are null in it. This is a lesser evil as the fixes to the navigation bar should make my custom renderer redundant, but breaking both navPage.On().SetHideNavigationBarSeparator and breaking NavigationBar.TitleTextAttributes forces me to roll back.

@Tommigun1980 Tommigun1980 added s/unverified New report that has yet to be verified t/bug 🐛 labels Feb 7, 2020
@pauldipietro pauldipietro added this to New in Triage Feb 7, 2020
@samhouts samhouts added this to the 4.5.0 milestone Feb 7, 2020
@samhouts
Copy link
Member

samhouts commented Feb 7, 2020

@Tommigun1980 Can you please attach a small project that demonstrates this issue? Thanks!

@samhouts samhouts added s/needs-info ❓ A question has been asked that requires an answer before work can continue on this issue. s/needs-repro ❔ This reported issue doesn't include a sample project reproducing the issue. Please provide one. labels Feb 7, 2020
@samhouts samhouts moved this from New to Needs Info in Triage Feb 7, 2020
@samhouts samhouts added this to To do in v4.5.0 Feb 13, 2020
@luccasclezar
Copy link

Here's the repro project: NavBarSeparator.zip

@samhouts samhouts removed s/needs-info ❓ A question has been asked that requires an answer before work can continue on this issue. s/needs-repro ❔ This reported issue doesn't include a sample project reproducing the issue. Please provide one. labels Feb 25, 2020
@jsuarezruiz
Copy link
Contributor

I have been testing in an example using 4.4 and in that version it is working.

Captura de pantalla 2020-02-26 a las 10 21 23

Captura de pantalla 2020-02-26 a las 10 21 03

Issue9456.zip

However, in 4.5-pre3 is not working (regression).

@jsuarezruiz jsuarezruiz removed the s/unverified New report that has yet to be verified label Feb 26, 2020
@jsuarezruiz jsuarezruiz moved this from New to Needs Estimate in Triage Feb 26, 2020
@Tommigun1980
Copy link
Author

Tommigun1980 commented Feb 26, 2020 via email

@skyapps-co
Copy link

Same issue here (SetHideNavigationBarSeparator(true)) no longer works with 4.5 on iOS 13. (It's fine on iOS < 13). I also had a custom renderer setting some NavigationBar properties to manually remove the separator and these do not work anymore either:

NavigationBar.SetBackgroundImage(new UIImage(), UIBarMetrics.Default); NavigationBar.ShadowImage = new UIImage();

@Tommigun1980
Copy link
Author

Imho it's a bit scary that new versions of the API get released with known regressions (referring to that 4.5.0 shred the pre-release tag). This makes it very hard to commit to an update. Imho a release should not be deemed ready if there are regressions as having such unstable releases makes it a hard sell to update, and also signals that things will break when people update.

@samhouts samhouts added this to To do in Sprint 167 Mar 3, 2020
@beeradmoore
Copy link
Contributor

Came across this as well, we are using what @skyapps-co uses to hide nav bar line. I need to either find another workaround or revert to 4.4 in the mean time until this can be fixed.

@Tommigun1980
Copy link
Author

Tommigun1980 commented Mar 6, 2020

Came across this as well, we are using what @skyapps-co uses to hide nav bar line. I need to either find another workaround or revert to 4.4 in the mean time until this can be fixed.

The following seems to work in 4.5.0.356 and 4.6.0.379-pre1 on the iOS versions I've tested on:

// TODO! remove this class once Xamarin fixes scroll navigation bar separator scroll edge hiding
using Xamarin.Forms;
using Xamarin.Forms.Platform.iOS;
using Xamarin.Forms.PlatformConfiguration.iOSSpecific;

using AppNameSpace.iOS.Renderers;

[assembly: ExportRenderer(typeof(Xamarin.Forms.NavigationPage), typeof(FixNavigationBarHiddenSeparatorRenderer))]
namespace AppNameSpace.iOS.Renderers
{
    public class FixNavigationBarHiddenSeparatorRenderer : NavigationRenderer
    {
        public override void ViewWillAppear(bool animated)
        {
            base.ViewWillAppear(animated);

            if (Element is Xamarin.Forms.NavigationPage navigationPage)
            {
                if (navigationPage.OnThisPlatform().HideNavigationBarSeparator())
                    NavigationBar.ScrollEdgeAppearance.ShadowColor = null;
            }
        }
    }
}

Hope it helps.

@beeradmoore
Copy link
Contributor

@Tommigun1980 I was unable to replicate your solution. I've attached a repro with your fix. Are you able to let me know if that works or not for you?

I also had to add an OS version check as ScrollEdgeAppearance was only added in iOS 13 and would crash when running on iOS 12.

NavBarLineTest.zip

@DjeeBay
Copy link

DjeeBay commented Mar 10, 2020

@beeradmoore maybe you could use NavigationBar.StandardAppearance.ShadowColor instead of NavigationBar.ScrollEdgeAppearance.ShadowColor.

@beeradmoore
Copy link
Contributor

@DjeeBay thanks! That workaround works. I want to go do some research into them both and see why they exist and how we should integrate into forms.

using System;
using UIKit;
using Xamarin.Forms;
using Xamarin.Forms.Platform.iOS;
using Xamarin.Forms.PlatformConfiguration.iOSSpecific;

[assembly: ExportRenderer(typeof(Xamarin.Forms.NavigationPage), typeof(NavBarLineTest.iOS.Renderers.FixNavigationBarHiddenSeparatorRenderer))]

namespace NavBarLineTest.iOS.Renderers
{
    public class FixNavigationBarHiddenSeparatorRenderer : NavigationRenderer
    {
        public override void ViewWillAppear(bool animated)
        {
            base.ViewWillAppear(animated);

            if (Element is Xamarin.Forms.NavigationPage navigationPage)
            {
                if (navigationPage.OnThisPlatform().HideNavigationBarSeparator())
                {
                    //NavigationBar.SetBackgroundImage(new UIImage(), UIBarMetrics.Default);
                    //NavigationBar.ShadowImage = new UIImage();
                    if (UIDevice.CurrentDevice.CheckSystemVersion(13, 0))
                    {
                        NavigationBar.StandardAppearance.ShadowColor = null;
                        //NavigationBar.ScrollEdgeAppearance.ShadowColor = null;
                    }
                }
            }
        }
    }
}

@beeradmoore
Copy link
Contributor

beeradmoore commented Mar 12, 2020

This above workaround doesn't work when using a MasterDetailPage. I'm also getting some odd nav bar color flickering (ill card up in a seperate issue soon).

Both issues may be from this commit based on its use of StandardAppearance, but I am not 100% certain.
271e41c#diff-efa21a065745b4e261efc4bf6a5d8784

EDIT: Workaround is to create a method to call the update fix code.

void FixNav()
{
    if (navigationPage.OnThisPlatform().HideNavigationBarSeparator())
    {
        if (UIDevice.CurrentDevice.CheckSystemVersion(13, 0))
        {
            NavigationBar.CompactAppearance.ShadowColor = null;
            NavigationBar.StandardAppearance.ShadowColor = null;
            NavigationBar.ScrollEdgeAppearance.ShadowColor = null;
        }
    }
}

And then call this whenever XF 4.5 calls UpdateBarBackgroundColor()

void HandlePropertyChanged(object sender, PropertyChangedEventArgs e)
{
	if (e.PropertyName == NavigationPage.BarBackgroundColorProperty.PropertyName)
	{
		FixNav();
	}
}
public override void ViewDidLoad()
{
	base.ViewDidLoad();
	FixNav();
}

@samhouts samhouts added this to To do in Sprint 168 via automation Mar 23, 2020
@samhouts samhouts moved this from To do to Continued in next sprint in Sprint 167 Mar 23, 2020
@markmccaigue
Copy link

I notice that #9952 appears to be a duplicate of this, and the fix looks to be released in 4.5.0-sr2

However we are still experiencing this issue on that version, including up to the latest prerelease of 4.6, which at the time of writing is 4.6.0-pre4.

Wondering if you could confirm if this is still considered an active issue? Thanks!

@samhouts samhouts moved this from To do to Continued in next sprint in Sprint 168 Apr 23, 2020
@michaeldimoudis
Copy link

Hi any update on this issue? Not fixed on latest stable 4.6 in my use case. Manually setting:

if (UIDevice.CurrentDevice.CheckSystemVersion(13, 0))
{
    NavigationBar.CompactAppearance.ShadowColor = UIColor.Clear;
    NavigationBar.StandardAppearance.ShadowColor = UIColor.Clear;
    NavigationBar.ScrollEdgeAppearance.ShadowColor = UIColor.Clear;
}

In a CustomNavigationPage : NavigationRenderer fixes the issue, but it's not reliable and I rather this bug to be fixed. As a result I can not upgrade from 4.4 to 4.5 or 4.6.

@ghost
Copy link

ghost commented May 11, 2020

I agree with @michaeldimoudis : this issue (as well as #9943) makes it impossible for us to update from 4.4. to anything newer. Any update on this issue?

@jfversluis
Copy link
Member

Would anyone be able to try the current nightly version? You can see how here.

It looks like this one should be fixed and is set to release in the next 4.6 service release. If that's the case, then it should definitely work in the currently nightly.

@ghost
Copy link

ghost commented May 11, 2020

@jfversluis One of our developers is going to do tests with the nightly build. I will keep you updated.

@studeruss
Copy link

@jfversluis I just tried the nightly version: 4.8.0.780-nightly, and before that also 4.6.0.369-nightly and the issue is not fixed yet, as we still see the separator

Please note that we use 'ios:NavigationPage.HideNavigationBarSeparator="True"' directly in XAML for pages that we want to hide the separator

@jfversluis
Copy link
Member

Alright, thanks for trying that out. Then it's clear that this is not fixed yet unfortunately

@michaeldimoudis
Copy link

I just tried 4.8.0.780-nightly and yep the it's not fixed yet. Nor is #9943

@samhouts samhouts added this to To do in Sprint 172 Jun 11, 2020
@samhouts samhouts added the 4.5.0 regression on 4.5.0 label Jun 17, 2020
@jsuarezruiz jsuarezruiz self-assigned this Jun 22, 2020
@jsuarezruiz jsuarezruiz moved this from To do to In progress in Sprint 172 Jun 22, 2020
@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Jun 22, 2020
@samhouts samhouts added this to In Progress in 4.7.0 Jun 22, 2020
@rmarinho rmarinho moved this from In progress to Ready for Review (Issues) in Sprint 172 Jun 24, 2020
@rmarinho rmarinho linked a pull request Jun 24, 2020 that will close this issue
2 tasks
@samhouts samhouts added this to In progress in Sprint 173 Jul 2, 2020
@samhouts samhouts moved this from Ready for Review (Issues) to Continued in next sprint in Sprint 172 Jul 6, 2020
@samhouts samhouts removed this from the 4.5.0 milestone Jul 6, 2020
@samhouts samhouts moved this from In Progress to Done in 4.7.0 Jul 13, 2020
@samhouts samhouts moved this from In progress to Done in Sprint 173 Jul 13, 2020
Triage automation moved this from Needs Estimate to Closed Jul 13, 2020
@samhouts samhouts removed this from Closed in Triage Aug 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4.5.0 regression on 4.5.0 a/navbar i/regression in-progress This issue has an associated pull request that may resolve it! m/high impact ⬛ p/iOS 🍎 platform-specifics t/bug 🐛
Projects
No open projects
4.7.0
  
Done
Sprint 167
  
Continued in next sprint
Sprint 168
  
Continued in next sprint
Sprint 172
  
Continued in next sprint
Sprint 173
  
Done
v4.5.0
  
To do
Development

Successfully merging a pull request may close this issue.