roubachof / Sharpnado.Tabs

Pure MAUI and Xamarin.Forms Tabs, including fixed tabs, scrollable tabs, bottom tabs, badge, segmented control, custom tabs, button tabs, bendable tabs...
MIT License
502 stars 64 forks source link

[MAUI][ANDROID] TabHostView Text Colors Don't Change Switching Light/Dark Mode #131

Open sk1llsh0t opened 2 weeks ago

sk1llsh0t commented 2 weeks ago

Platform (please complete the following information):

OS: [Android] Device: Tab Active 3 [Android API Level 31] Sdk Version: Min: 30 Target: 34 MAUI: [NET 8.0][8.0.91]

I notice the text colors for selected and unselected tabs aren't switching when the dark/light mode changes. The background color changes and the segmentedoutlinecolor changes. Something isn't working right with the text colors though when switching dark/light mode.

This issue was introduced after the fix for this issue (https://github.com/roubachof/Sharpnado.Tabs/issues/127#)

I'm currently on sharpnado.tabs 3.1.1

To Reproduce Steps to reproduce the behavior:

- Create an app with the Tabs control
- In the Resources/Styles/Styles.xaml, add styling for the various tab components with AppThemeBindings for light and dark modes
- Run the app and navigate to the page with the tab control
- Then, from the notification tray, change the theme from light to dark or vice versa.
- Dismiss the notification tray after the theme has switched so you are back in the app.
- You'll notice as you switch the dark/light mode, the text colors in the tabhostview aren't changing

Styles used in my Style.xaml file: (of course change the color StaticResources to match your colors)

 <Style TargetType="sho:TabHostView" x:Key="TabHostView">
        <Setter Property="BackgroundColor" Value="{AppThemeBinding Light={StaticResource LightBackground02}, Dark={StaticResource DarkBackground02}}" />
        <Setter Property="SegmentedOutlineColor" Value="{AppThemeBinding Light={StaticResource Gray950}, Dark={StaticResource Gray200}}" />
        <Setter Property="CornerRadius" Value="10" />
        <Setter Property="TabType" Value="Fixed" />
        <Setter Property="IsSegmented" Value="True" />
        <Setter Property="Orientation" Value="Horizontal" />
        <Setter Property="SegmentedHasSeparator" Value="True" />
    </Style>

    <Style TargetType="sho:UnderlinedTabItem" x:Key="UnderlinedTabItem">
        <Setter Property="SelectedTabColor" Value="{AppThemeBinding Light={StaticResource Primary}, Dark={StaticResource PrimaryLight}}" />
        <Setter Property="UnselectedLabelColor" Value="{AppThemeBinding Light={StaticResource LightSecondaryText}, Dark={StaticResource DarkSecondaryText}}" />
    </Style>

    <Style TargetType="sho:BadgeView" x:Key="BadgeView">
        <Setter Property="Background" Value="{StaticResource Secondary}"></Setter>
    </Style>

Expected Behavior: Tab text colors change to their respective dark/light theme color

roubachof commented 2 weeks ago

Look like a good first issue to me 😄

sk1llsh0t commented 2 weeks ago

Repro: https://github.com/sk1llsh0t/TabVmBindingRepro

I created this repro for another issue but it demonstrates this issue as well. it is more obvious if the device is first in dark mode and then you change to light mode.

roubachof commented 2 weeks ago

I tried to fix that, but no idea why the property changed is not triggered on the tab item's property...

sk1llsh0t commented 2 weeks ago

It was working before 3.1.0/3.1.1.

roubachof commented 2 weeks ago

Oh cause we switched to .net 8 maybe

Le jeu. 24 oct. 2024, 14:25, sk1llsh0t @.***> a écrit :

It was working before 3.1.0/3.1.1.

— Reply to this email directly, view it on GitHub https://github.com/roubachof/Sharpnado.Tabs/issues/131#issuecomment-2435154211, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAERXJ2EA67W747F7J2MJMDZ5DRNFAVCNFSM6AAAAABQOW3VCKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMZVGE2TIMRRGE . You are receiving this because you commented.Message ID: @.***>

sk1llsh0t commented 1 week ago

I believe I know what is happening. Right now, you have an OnPropertiesChanged override that is handling all the property changes. It uses a switch statement to know which property changed and take the appropriate action. In my experience moving our app from xamarin forms to maui (.net 8.0), this no longer works consistently. Some properties it will fire for but others, not.

The way to do it is when instantiating the bindableproperty, there is a parameter for onpropertychanged which you can define a callback for. if you pass the method in through that parameter directly to the bindableproperty, it will work.

This is definitely a bug in maui that has not been addressed yet apparently. Give that a try and see if it starts working again.

janusw commented 1 week ago
  • In the Resources/Styles/Styles.xaml, add styling for the various tab components with AppThemeBindings for light and dark modes

Does this work at all for you? Whenever I try to style the tabs via Styles.xaml I get a strange exception (with 3.1.0 and 3.2.0 on Android):

System.NullReferenceException: Object reference not set to an instance of an object.
  at at Sharpnado.Tabs.TabHostView.UpdateTabType()
  at at Sharpnado.Tabs.TabHostView.OnPropertyChanged(String propertyName)
sk1llsh0t commented 1 week ago

it does work for me. the repro project i have posted above uses styles.xaml (https://github.com/sk1llsh0t/TabVmBindingRepro)

sk1llsh0t commented 1 week ago

So basically, you eliminate this method along with any other classes that might override OnPropertyChanged in the same way https://github.com/roubachof/Sharpnado.Tabs/blob/07f3072e2496bd6c76e6b3e7250c61767524ff69/Tabs/Tabs/UnderlinedTabItemBase.cs#L39

Then, you modify your bindableproperties to handle the property change. Example:

public static readonly BindableProperty SelectedTabColorProperty = BindableProperty.Create(
            nameof(SelectedTabColor),
            typeof(Color),
            typeof(TabItem),
            Colors.Magenta,
            propertyChanged: OnSelectedTabColorChanged 
);

private static void OnSelectedTabColorChanged(BindableObject bindable, object oldValue, object newValue) 
{ 
        // Same logic from the old OnPropertyChanged       
        UpdateColors();
}
sk1llsh0t commented 1 week ago

Any progress/update on this one?

roubachof commented 1 week ago

nope. Already tried what you suggested unfortunately

sk1llsh0t commented 1 week ago

The issue was introduced in this commit: (https://github.com/roubachof/Sharpnado.Tabs/commit/c60990f72d523c38df60776d7a0004fff791256d)

In the TabHostView class, the frame control was changed to a border. Changing it back to a frame makes the app theme colors work again.

To figure it out, i grabbed the previous good commit, and started adding each changed code section 1 at a time until it broke. Since the border was the last thing i thought would break something like this, it was the last remaining thing to check. Anyway, there you go.

janusw commented 1 week ago

Does this work at all for you? Whenever I try to style the tabs via Styles.xaml I get a strange exception (with 3.1.0 and 3.2.0 on Android):

System.NullReferenceException: Object reference not set to an instance of an object.
  at at Sharpnado.Tabs.TabHostView.UpdateTabType()
  at at Sharpnado.Tabs.TabHostView.OnPropertyChanged(String propertyName)

FYI: I managed to reproduce this with the sample app and opened issue #138 for it.

roubachof commented 1 week ago

@sk1llsh0t nice job ! I will fix #138 and see what's what

roubachof commented 1 week ago

hopefully fixed by v3.2.1

sk1llsh0t commented 1 week ago

I just tried 3.2.1 and it does not fix this issue unfortunately.

sk1llsh0t commented 1 week ago

Any update on this one?

roubachof commented 1 week ago

I won't have any time on this one. I guess you are very close to find the issue after your recent findings:)

roubachof commented 1 week ago

Meaning: if you find the issue I will make a new package ASAP

sk1llsh0t commented 1 week ago

My opinion is something is broken in the border control that isn't allowing the property events to fire on child elements when dark/light mode changes. The only solution I can come up with is to revert the border back to a frame. Unfortunately, the frame control is flagged as obsolete in .net 9. It is possible that the maui folks need to take a look at this issue in the border control.

Anyway, i have a forked branch where i've reverted the border back to a frame. if you are ok with that, i can submit a pull request. This can be a temporary solution until the border is fixed.

Let me know.

sk1llsh0t commented 1 week ago

I don't have any more time to give to this much less wait on Microsoft to come up with a fix to their border control. I may wait until .Net 9 is released to see if that fixes it. Otherwise, without the ability to switch the app theming colors correctly, I may need to roll my own tabview or something.

You've done great work with this control. It is just a shame that something out of your control is causing this breaking issue.

roubachof commented 1 week ago

as a matter of fact you could intercept the Theme change event and set the color accordingly until MS fixes the issue.

roubachof commented 1 week ago

Well for them to fix the issue we would need to open a bug I guess xd

sk1llsh0t commented 1 week ago

One other thing i noticed, if i set IsSegmented=False (no border), the colors work. then if i add a border around the tabhostview, the colors continue to work. So it looks like the issue is not there if xaml is used.