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

[Bug] AutomationProperties on Button still visible on Android if Text Property is empty #7875

Closed
MathewLC opened this issue Oct 8, 2019 · 26 comments
Assignees
Labels
a/a11y 🔍 in-progress This issue has an associated pull request that may resolve it! p/Android t/bug 🐛

Comments

@MathewLC
Copy link
Contributor

MathewLC commented Oct 8, 2019

Description

This error is hapenning in the actual version of xamarin forms.

Basic Information

  • IDE: Visual Studio 2019

  • Platform Target Frameworks:

    • Android: Android 10 (API Level 29)
  • Nuget Packages: Xamarin.Forms (4.2.0.848062)

  • Tested Devices: Emulators with Android 10 and 9. Nokia x6 with Android 9.

Screenshots

image

Reproduction Link

AppTeste.zip

@jfversluis
Copy link
Member

Seems to happen indeed, only on Android. What I somehow suspect is that we somehow, somewhere assign this to a visible property whenever that is empty? Need to investigate a bit on that.

As a workaround you could enter a space in your Text property for now.

@jfversluis jfversluis removed the s/unverified New report that has yet to be verified label Oct 8, 2019
@magoolation
Copy link
Contributor

I'd like to work om this as I'm focused on accessibility improvements on Xamarin.Forms

@jfversluis
Copy link
Member

By all means, please go ahead! I'll assign it to me for now so no one will snatch it from you. If, for some reason, you can't finish it please let me know :)

@jfversluis jfversluis self-assigned this Oct 8, 2019
@MathewLC
Copy link
Contributor Author

MathewLC commented Oct 9, 2019

If help you guys, I run into another issue related to this. It seems like the properties are only hide but definily is on the UI. In the sample I'll send, the same button with and without AutomationProperties, has different width value as you can see in the screenshot.

ScreenShot

image

Reproduction Link

AppTeste.zip

@jfversluis
Copy link
Member

I'm not sure what you're trying to say. You mean using the AutomationProperties are impacting the width?

@hartez
Copy link
Contributor

hartez commented Oct 9, 2019

I'm not sure what you're trying to say. You mean using the AutomationProperties are impacting the width?

We had a problem like that last year; maybe this is a similar situation? #2580

@MathewLC
Copy link
Contributor Author

MathewLC commented Oct 9, 2019

Exactly. In the code that's in the sample I've sended, you can see that the two buttons that reproduce this images, are equals with the only difference being the automationproperties in the last one.

See the folling code:

        <!--This don't have extra width because don't have AutomationProperties --> 
        <Button Text="Tiny"
                VerticalOptions="CenterAndExpand"
                HorizontalOptions="Center"
                BackgroundColor="Blue"/>
    
        <!-- This have extra width because of AutomationProperties -->
        <Button Text="Tiny" 
                AutomationProperties.Name="Name"
                AutomationProperties.HelpText="Help Large Text "
                VerticalOptions="CenterAndExpand"
                HorizontalOptions="Center"
                BackgroundColor="Blue"/> 

@hartez
Copy link
Contributor

hartez commented Oct 9, 2019

I wonder if this is showing up again because the fast renderers are the default now? Looks like Rui fixed this in the older LabelRenderer; maybe the same fix needs to be applied to the fast renderer: https://github.com/xamarin/Xamarin.Forms/pull/4433/files#diff-c754c889f501d0bbd657ea1ca0ad3370

@MathewLC MathewLC changed the title [Bug] AutomationProperties on Button still visible on Android if Text Propertie is empty [Bug] AutomationProperties on Button still visible on Android if Text Property is empty Oct 9, 2019
@MathewLC
Copy link
Contributor Author

MathewLC commented Oct 9, 2019

I think it's the same @hartez !

@MathewLC
Copy link
Contributor Author

MathewLC commented Oct 9, 2019

I wonder if this is showing up again because the fast renderers are the default now? Looks like Rui fixed this in the older LabelRenderer; maybe the same fix needs to be applied to the fast renderer: https://github.com/xamarin/Xamarin.Forms/pull/4433/files#diff-c754c889f501d0bbd657ea1ca0ad3370

I can confirm that using the flag "UseLegacyRenderers", the UI show as expected!

@jfversluis
Copy link
Member

Nice catch @hartez!

@magoolation this should be an easy fix then :) are you able to implement this anytime soon? Else to get this bug out of the way ASAP I can do it for you

@magoolation
Copy link
Contributor

magoolation commented Oct 10, 2019 via email

@jfversluis
Copy link
Member

Awesome! Have a great conference and looking forward to your PR then

@jfversluis
Copy link
Member

@magoolation Not trying to put pressure on you, sorry! But did you make any progress with this yet? Please let us know if you need help or want us to take over. Thanks!

@magoolation
Copy link
Contributor

magoolation commented Oct 28, 2019 via email

@jfversluis
Copy link
Member

jfversluis commented Oct 28, 2019

Did you see this comment from a little bit earlier? You can find it here: #7875 (comment)

Although I do see they are talking about a Label and not a Button there... So this code might be needed in the LabelRenderer in the FastRenderers folder but also on the button?

@MathewLC
Copy link
Contributor Author

I didn't test the label. But I'm sure the button error was fixed in old render.
It seems like this is the right commit in which the fix was done:
ff13c49 @jfversluis @magoolation

Did you see this comment from a little bit earlier? You can find it here: #7875 (comment)

Although I do see they are talking about a Label and not a Button there... So this code might be needed in the LabelRenderer in the FastRenderers folder but also on the button?

@jfversluis
Copy link
Member

Thanks @MathewLC !

Let me know if you can make any progress with this @magoolation. Else I will have a go at this to get this a11y bug out of the way!

@magoolation
Copy link
Contributor

magoolation commented Oct 28, 2019 via email

@samhouts samhouts added the in-progress This issue has an associated pull request that may resolve it! label Nov 5, 2019
@czuck
Copy link

czuck commented Nov 21, 2019

I can confirm that this is happening with Labels as well. Here is my test code:

        <StackLayout
            VerticalOptions="FillAndExpand"
            Orientation="Vertical"
            BackgroundColor="Yellow"
            Spacing="10" >
            <Label x:Name="FirstLabel"
                Text="First Label"
                HorizontalOptions="Center"
                Font="12" 
                   BackgroundColor="Aqua"/>
            <Label x:Name="SecondLabel"
                Text="Second Label"
                HorizontalOptions="Center"
                Font="12" 
                   BackgroundColor="BlueViolet"
            AutomationProperties.Name="This is a really long Automation Properties Name, It's so long it takes up two lines"/>
            <Label x:Name="ThirdLabel"
                Text="Third Label"
                HorizontalOptions="Center"
                Font="12" 
                   BackgroundColor="ForestGreen"/>
            <Label x:Name="FourthLabel"
                Text="Fourth Label"
                HorizontalOptions="Center"
                Font="12" 
                   BackgroundColor="HotPink"
                   AutomationProperties.Name="Short Name"/>
        </StackLayout>
        public MainPage()
        {
            InitializeComponent();
            AutomationProperties.SetName(FirstLabel, "This is a really long Automation Properties Name, It's so long it takes up two lines");
            AutomationProperties.SetName(ThirdLabel, "Short Name");
        }

image

@MathewLC
Copy link
Contributor Author

@czuck I think you can wait and see if the pull request that reference this bug fix it in the next version of xamarin forms(4.4.0). If not you can open another Issue and reference this.

@czuck
Copy link

czuck commented Dec 9, 2019

Labels are working properly with 4.4.0-pre3! Thanks!

@jfversluis
Copy link
Member

Thanks for letting us know @czuck ! @MathewLC can you confirm and update this issue appropriately?

@MathewLC
Copy link
Contributor Author

@jfversluis I tested with the last release version of forms and in fact labels are working but buttons aren't. You guys didn't merge this yet, am I right?

@jfversluis
Copy link
Member

Whoops, no the PR is still open and I see it was just waiting for my review 😱 sorry about that! I'll see to it that I do that ASAP. Thanks for the ping @MathewLC !

jfversluis pushed a commit that referenced this issue Dec 17, 2019
…ties (#8368)

* Tryed to reproduce the same correction applied to the default implementation

* Added repro sample

* Fixed build error on iOS
@samhouts
Copy link
Contributor

closed by #8368

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
a/a11y 🔍 in-progress This issue has an associated pull request that may resolve it! p/Android t/bug 🐛
Projects
None yet
Development

No branches or pull requests

6 participants