Updated StatusBarBehavior to support defining which lifecycle event to use in applying the changes

[Cybrosys]

Description of Change

• Updated the StatusBarBehavior to use the Page's NavigatedTo event to set the desired color and style.
• Sample was updated to showcase that back navigation now sets the desired color and style.
• Unit test/fact was added to confirm expected exception on NavigatedTo.
  • Had to use reflection to invoke OnAttachedTo so the behavior would subscribe to the Page's NavigatedTo event.

Linked Issues

• Fixes [BUG] StatusBarBehavior not reflected on back navigation #1194
• Fixed [BUG] StatusBarBehavior StatusBarStyle not updating correctly on Android 10 #1466

PR Checklist

• Has a linked Issue, and the Issue has been approved(bug) or Championed (feature/proposal)
• Has tests (if omitted, state reason in description)
• Has samples (if omitted, state reason in description)
• Rebased on top of main at time of PR
• Changes adhere to coding standard
• Documentation created or updated: Added in some detail about the new ApplyOn property MicrosoftDocs/CommunityToolkit#378

Additional information

There was a mix of tabs and spaces in the StatusBarBehaviorPage.xaml file. Looking at the majority of the other xaml files they were using spaces instead of tabs, so I merely updated the xaml file to match the others and only have spaces.

[Cybrosys]

@dotnet-policy-service agree

[Cybrosys]

Is the toolkit build failing due to the unit tests failing? I saw that there were several failing Color-unit tests due to bad string comparison, for example:

Expected: CMYKA(100%,100%,0%,0%,1)
Actual:   CMYKA(100 %,100 %,0 %,0 %,1)

I don't know the policy in this case, if I should fix the unit tests or ignore them as they are out of scope for this PR?

[pictos]

@Cybrosys Thanks for your contribution, but this doesn't feel right. At first glance, it matches the behavior that you expect, but it breaks the API for PlatformBehavior, by design isn't intended to run every time you navigate in an implicit way. So we need to think of another fix here, maybe adding a boolean for the user to choose when he wants to trigger the Color change during navigation.

Not sure if you followed all the discussion on #1194 or just referenced it as a fixed issue, but there're implementation details and guidelines, for example, do Apple and Google recommend an app to change the status bar color very often? As a framework we don't want to lead devs to the not recommended path, if you need it for your project you should implement it on your project.

About the Tabbar navigation on Shell triggering the PlatformBehavior, that's because an implementation detail on Shell that came from XF, where the Tabs are recreated every time you navigate to it, so that means the page will be created and behavior will be attached triggering the OnAttached method, in other words, there's not to do with the behavior itself and something that, AFAIK, the team wants to fix in future releases

[Cybrosys]

@pictos I understand. I merely created this PR as I saw that @bijington had requested that one be created back in June during the discussions on #1194. I was directed to that issue by @cat0363 from my own reported issue #1466.

The changes are based on what @cat0363 suggested.

From my point of view, if I have a page that is configured with a specific status bar color and style, I would expect that to be respected regardless of which navigational pattern was involved in the user getting there. Changing the colors and styles might deviate from Apple's or Google's recommendations, but in the end is that not a choice left up to the developer?

Adding a boolean or enumeration/flags to control when the color and style should change sounds good to me. Any suggestions?

[bijington]

If it is a recommendation to not keep changing the status bar colour then I would argue that we already make it possible to go against this recommendation given the nature of the StatusBarBehavior - developers can set it at the page level rather than an application level.

The above is why I think it makes sense for the default behavior to set the status bar color to match whatever the current page is set to.

I am all for the idea of the change :)

[pictos]

About the recommendation of not changing the status bar a lot, I'm not sure since has been a long time since the last app that I released to the stores, but I remember that it should be something stable for the most part of the app. But we can't protect devs from themself all the time but is good to not provide an easy way for them to hurt themself.

@Cybrosys I hope my message didn't sound rude, English isn't my primary language, and I just want to have as much information as possible about the concerns around this change.

If the team thinks it's fine to add the flag, I would say it should be enough to have a boolean property for that. But we would need to document the Shell behavior since they will be generated every time devs should handle that by themself

[Cybrosys]

@pictos No, not at all. And the same for me and how I replied.

If we look at styling the status bar. I have never seen that as something separate from the current view. When I build native Android apps each Activity has a Theme, each theme can override how the status bar looks. The Theme will be applied regardless of how an Activity was presented or navigated back to,

Maui does not expose any styling options for the status bar, which is why the StatusBarBehavior was created; please correct me if I'm wrong. In one sense I would wish that it was supported in Maui, but since it is not I would expect the StatusBarBehavior to give me the expected behavior as if it was supported in Maui.

I can understand that StatusBarBehavior breaks the API for PlatformBehavior, but I do not fully understand the hesitancy of changing the status bar color and style as it is supported natively on Android per Theme and Activity. iOS is a slightly different matter as the status bar is transparent so the navigation bar color becomes the status bar color. So I can understand how overly manipulating the status bar on iOS might be frowned upon by Apple.

If the concern is the amount of calls to alter the status bar, perhaps the existing status bar color and style could be checked before making the call? When an instance of StatusBarBehavior is created, the current color and style is not retrieved from the current platform. On Android the current status bar color could be retrieved using getStatusBarColor method on Window.

So, if the StatusBarBehavior was a bit smarter and actually checked the current color and style, it would remove unnecessary calls to alter the status bar.

Or?

[pictos]

My concern is not about the number of times the method will be called, but how many times the statusBar color will change. If we can confirm that isn't something that Apple is against, we can implement it.

In any case, the best implementation is using the boolean flag, that way it's not a breaking change, and users can opt-in to that behavior.

[Cybrosys]

Any suggestion to what the flag/property should be called and if it should be a bindable property?

• SetColorAndStyleWhenNavigatedTo
• SetColorAndStyleOnAppearing
• ActivateWhenNavigatedTo
• ActivateOnAppearing
• ...

Or should it be an enum property Mode(?) where the enum would have two values?

• Default/Once
• Always/OnAppearing

[KSemenenko]

I did something like this for XF, there is no way except update status bar color each time you perform navigation

[cat0363]

@Cybrosys , I remember that the discussion on Issue #1194 was inconclusive.
For this reason, I did not create a PR. This is because it is desirable to deal with things as they should be.
I agree with this suggestion.

Rather than making destructive changes, I think it's better to leave the timing of changes up to the users.
I think a compromise solution is to set up a flag as @pictos says. The code I showed in Issue #1194 was missing that.

Maybe we need to think about what it means to change the color of the status bar. I believe that by doing so,
we will be able to see what it really should be.

[cat0363]

@Cybrosys , I forgot to mention it, but it sounds like it's a device-specific issue.
Is this really the solution?

[Cybrosys]

I can think of a couple of normal scenarios where you might want to change the status bar color and style, one would be showing an onboarding view without a navigation bar or some view where you might want the contents to extend in under the status bar. Once the user navigates back from those views you'd want the status bar restored to its default state for the app.

Since the current implementation of the StatusBarBehavior does not restore the state or reapply it, I would have to implement this myself in the code-behind, which defeats the purpose of the StatusBarBehavior a bit. I might also be expecting too much of the behavior.

If my app was never going to change the status bar color I could update the Android style or use the static methods on the StatusBar once, which the StatusBarBehavior solves today. But if I want anything more than that we get into these scenarios where some expectations are not met, resulting in the developer thinking that there's a bug with the behavior.

[bijington]

It sounds like something non breaking is a good way to go. Rather than a Boolean property to drive this what does everyone think to an enum that allows a developer to define which lifecycle event this behavior gets applied? e.g. OnNavigatedTo, OnAppearing, etc.

[brminnick]

Rather than a Boolean property to drive this what does everyone think to an enum that allows a developer to define which lifecycle event this behavior gets applied? e.g. OnNavigatedTo, OnAppearing, etc.

Enum makes sense, since it'll give us the opportunity to add additional lifecycle states in the future. Someday, maybe (hopefully!) the MAUI team will give us us OnLoaded or WillAppear.

[pictos]

@brminnick just to confirm and move forward with this implementation, are we going with enum instead of a boolean?

[brminnick]

Yup! I vote for an enum.

It's a bit overkill today, but it'll allow us the flexibility to add additional overloads/implementations in the future without breaking changes.

[pictos]

@Cybrosys do you have time to implement this feature? Let me know if you would like to discuss the API design or something before doing some code.

public enum NameThatYouChoose
{
    None,
    PageNavigation
}

For now, we should have two values, one for the default (actual) behavior and the new one. You can use other names since I'm terrible at naming things.

[Cybrosys]

@pictos yes I have the time to implement the feature.

Naming things is also hard for me, and doubly so for MAUI as I am not familiar with the current naming strategy of classes or enums. With that said, I would greatly appreciate suggestions with regards to naming. As I see it there are three areas:

• The name of the property on StatusBarBehavior.
• The name of the enum (will be easier to come up with once we decide on the property name).
• The enum values (will be easier to come up with once we decide on the property name).

An initial idea regarding to the property name was Behavior, as it is the behavior of the behavior... but, since the name of the class already contains that word it does not feel correct:
StatusBarBehavior.Behavior = ...;

Other property name ideas are:

• Mode
• Reapply(On/When)
• Trigger(On/When)

There is also the question of what to do with the unit test that I added (that is using reflection). Should I remove it or is there some way that I could trigger the OnAttachedTo to achieve the same result?

[bijington]

What if we use the following:

• property name: ApplyOn
• enum name: LifecycleEvent
• value name: OnNavigatedTo

Then the code would read:

<toolkit:StatusBarBehavior Color="Orange" ApplyOn="LifecycleEvent.OnNavigatedTo"

Or

StatusBarBehavior behavior = new();

behavior.Color = Colors.Orange;
behavior.ApplyOn = LifecycleEvent.OnNavigatedTo;

Thoughts?

And I would default the value to be whatever lifecycle event we currently base it off.

[Cybrosys]

@bijington I like those names.

The enum LifecycleEvent sounds like something that could exist in the core MAUI project and that makes me wonder where we would put it in the CommunityToolkit that would make sense and hopefully not cause any naming conflicts, perhaps a different name more related to the StatusBarBehavior would be something to consider?

Another thought is also if OnAttachedTo and OnNavigatedTo are lifecycle events? If I think of lifecycle events I think of created, paused, resumed and destroyed; OnAttachedTo could be treated as related to the created-lifecycle event.

What about StatusBarEvent?

[bijington]

You make a good point. I was hoping we could go for a general set of lifecycle events but I was only focussed on Page related ones.

I'll try and have a think today

[Cybrosys]

How about naming it based on the property name ApplyOn:

public enum StatusBarApplyOn
{
    OnAttachedTo,
    OnNavigatedTo
}

If you see that it might be used for other behaviors then maybe:

public enum BehaviorApplication
{
    OnAttachedTo,
    OnNavigatedTo
}

[bijington]

How about naming it based on the property name ApplyOn:

public enum StatusBarApplyOn

{

    OnAttachedTo,

    OnNavigatedTo

}

If you see that it might be used for other behaviors then maybe:

public enum BehaviorApplication

{

    OnAttachedTo,

    OnNavigatedTo

}

I'm happy with the more specific name for now. I don't think the generic would work for all scenarios because OnNavigatedTo is a page event whereas OnAttachedTo is a behavior event. On that note should we include that detail in the name? So it would be something like;

public enum StatusBarApplyOn
{
    OnBehaviorAttachedTo, 
    OnPageNavigatedTo
}

[pictos]

From my point of view, the OnAttachedTo will lead devs to a pitfall. Every behavior will be triggered when it's attached, but with your enum, I can see devs thinking they can trigger the behavior only on OnPageNavigatedTo and that will not be true... Also, we need to think in a scenario where devs can opt-in for more than one value on that enum. I would rename the OnAttachedTo (or any variation of that) to Default

[bijington]

You make a good point re: OnAttachedTo. And also with the multiple values - we could mark the enum with Flags and handle it that way. Not sure how easy it is set in XAML that way.

I prefer naming the default value to represent the lifecycle event it relates to and we just make that the default value of the property. The name Default won't mean anything to a developer unless they look it up which feels wrong.

[Cybrosys]

It is possible to use flags in XAML:
ApplyOn="OnBehaviorAttachedTo,OnPageNavigatedTo"

...and we can have Default and OnBehaviorAttachedTo:

[Flags]
public enum StatusBarApplyOn
{
	/// <summary>
	/// Apply color and style when the behavior has been attached to the page.
	/// </summary>
	Default = OnBehaviorAttachedTo,

	/// <summary>
	/// Apply color and style when the behavior has been attached to the page.
	/// </summary>
	OnBehaviorAttachedTo = 0,

	/// <summary>
	/// Apply color and style when the page has been navigated to.
	/// </summary>
	OnPageNavigatedTo = 1,
}

[pictos]

Actually, the thing is, that the Default/AttachedTo will always happen, so I believe we can omit it.

[bijington]

Do we want to include OnPageSizeChanged as a possible option also? Although I realise we only use this by default for iOS.

Although just to circle back to the original question we had discussed as part of this issue, if we don't need the default behavior can we just fix the behavior to apply the status bar behavior in OnNavigatedTo and not bother with the extra enum property?

[Cybrosys]

Actually, the thing is, that the Default/AttachedTo will always happen, so I believe we can omit it.

I added the below check in OnAttachedTo, so unless I misunderstand the statement, it will only happen if you specify it:

if (ApplyOn == StatusBarApplyOn.OnBehaviorAttachedTo)
{
	StatusBar.SetColor(StatusBarColor);
	StatusBar.SetStyle(StatusBarStyle);
}

I do not understand how we can omit Default/AttachedTo from the enum as that would make it a single value enum. If we were to support that then it would have to be a nullable enum? If we're talking about a nullable single value enum then that almost sounds like what was suggested earlier of adding a boolean property:
ApplyOnNavigatedTo

[bijington]

I can't speak for @pictos here but I think he means we don't see a need for using that event in general and therefore could remove the enum value and also the current code to apply the logic then.

If the above is the case then I would return to my earlier question around whether your original fix was the right one (I think it is) and we forget about the enum.

In the other hand I do quite like the idea of introducing an enum like this in future to make the behaviors more configurable.

[pictos]

Friends, just to give feedback, I'll take a look at this in more detail this weekend/next week. I'm very focused on my .net conf talk right now (:

But I've some ideas on how we can approach this, just need to do a PoC and share with you

[bijington]

Where did we get with this? @pictos did you manage to play with that PoC?

[pictos]

@bijington not yet... There's a lot of stuff on my plate and not too much time. But I'll work on this in order to have something right after the holidays. I'll update you all asap

[bijington]

I am really sorry we have taken so long on this PR. I would like to try and get it over the line now, I think we have two options:

1. Use the original idea of applying this behaviour always when the page is navigated to.
2. Continue with the enum and apply it based on what the user has defined.

I think I like option 2 as it makes it more configurable although I would question when we really need the OnAttachedTo logic. Thoughts anyone?

[Cybrosys]

@bijington the only argument would be to not break the current and/or expected behavior.

@pictos mentioned he had an idea of how to approach it, so maybe he has or will have some spare time to pursue those ideas?

[bijington]

Ok that's fair. I think I'd be happy to move forward with the current approach. I know how busy Pedro is and I don't want to add to his list of things to do.

[bijington]

I'm happy to go ahead with this

We have address this with the ApplyOn property

[bijington]

@Cybrosys thank you for making the effort to provide this improvement! It is greatly appreciated