New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] Remove the ". " on empty labels (Accessibility) on Fastrenderers #915

Merged
merged 5 commits into from Jun 22, 2017

Conversation

Projects
None yet
6 participants
@samhouts
Member

samhouts commented May 10, 2017

Description of Change

This is an alternative to #904 (and revision to #801). This will clarify and simplify the code a bit more.

Note that this code will be moved to extension methods if #889 is accepted.

Also adds a UI test to prevent recurrence.

Also removed the nullable parameters in the AutomationPropertiesProvider that are no longer necessary.

Bugs Fixed

API Changes

None

Behavioral Changes

None

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
Show outdated Hide outdated ...rin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
else
separator = ". ";
return $"{name}{separator}{helpText}";

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix May 11, 2017

Member

1/ Are you sure you mean IsNullOrWhiteSpace and not IsNullOrEmpty ?

2/ Should be:

if (string.IsNullOrWhiteSpace(name)
    return helpText;
if (string.IsNullOrWhiteSpace(helpText)
    return name;

return $"{name}. {helpText}"

so you don't concatenate empty strings (this 2/ is only valid if you don't care about whitespaces

@StephaneDelcroix

StephaneDelcroix May 11, 2017

Member

1/ Are you sure you mean IsNullOrWhiteSpace and not IsNullOrEmpty ?

2/ Should be:

if (string.IsNullOrWhiteSpace(name)
    return helpText;
if (string.IsNullOrWhiteSpace(helpText)
    return name;

return $"{name}. {helpText}"

so you don't concatenate empty strings (this 2/ is only valid if you don't care about whitespaces

Show outdated Hide outdated ...rin.Forms.Platform.Android/FastRenderers/AutomationPropertiesProvider.cs
textView.Hint = !string.IsNullOrWhiteSpace(value) ? value : _defaultHint;
return true;
}
static string ConcatenateNameAndHelpText(Element Element)

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix May 11, 2017

Member

I'd AggressiveInline this

@StephaneDelcroix

StephaneDelcroix May 11, 2017

Member

I'd AggressiveInline this

Show outdated Hide outdated Xamarin.Forms.Platform.Android/VisualElementRenderer.cs
@@ -411,6 +411,20 @@ protected virtual bool SetHint()
return true;
}
static string ConcatenateNameAndHelpText(Element Element)

This comment has been minimized.

@StephaneDelcroix

StephaneDelcroix May 11, 2017

Member

do we need this declared twice ?

@StephaneDelcroix

StephaneDelcroix May 11, 2017

Member

do we need this declared twice ?

@BradChase2011

This comment has been minimized.

Show comment
Hide comment
@BradChase2011

BradChase2011 May 17, 2017

Contributor

@samhouts Sam, do you know how far away this is from being accepted? Merge wise, do we need to merge this with other PRs or are we good just merging this in for the time being until its accepted?

Contributor

BradChase2011 commented May 17, 2017

@samhouts Sam, do you know how far away this is from being accepted? Merge wise, do we need to merge this with other PRs or are we good just merging this in for the time being until its accepted?

samhouts added some commits May 10, 2017

[Android] Concatenate Name/HelpText better
Also remove some parameters that were added to workaround an issue that
no longer exists.
Listen to Stephane
he's usually right

@pauldipietro pauldipietro referenced this pull request Jun 14, 2017

Closed

[Android] Respect use of TextFormatted on Button #972

4 of 4 tasks complete

@rmarinho rmarinho merged commit f80a5e0 into master Jun 22, 2017

8 of 9 checks passed

Android-UITests-Stable-PreAppCompat Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 (Pre-AppCompa…
Details
Android-UITests-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 : Tests passe…
Details
Android-UITests-Stable-LegacyRenderers Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run Android 6.0.1 (Legacy Rende…
Details
OSX-Debug-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: OSX Debug : Running
Details
Windows-Debug-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Debug : Tests passed: 3801, ignored: 10
Details
Windows-Release-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: Windows Release Unit Tests : Tests passed: 3801, ignored: 10
Details
iOS10-UITests-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS10 : Tests p…
Details
iOS8-UITests-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified IOS8 : Tests pa…
Details
iOS9-UITests-Stable Finished TeamCity Build Xamarin.Forms :: Debug :: Stable - Cycle 9 :: UI Tests :: OSX Test Cloud Package - Run iOS Unified iOS9 : Tests pa…
Details

rmarinho added a commit that referenced this pull request Jun 22, 2017

[Android] Remove the ". " on empty labels (Accessibility) on Fastrend…
…erers (#915)

* [Android] Concatenate Name/HelpText better

Also remove some parameters that were added to workaround an issue that
no longer exists.

* Add repro

* Listen to Stephane

he's usually right

* oops, didn't save

@samhouts samhouts deleted the android-helptext branch Jun 22, 2017

assemhakmeh added a commit to assemhakmeh/Xamarin.Forms that referenced this pull request Jul 28, 2017

Merge branch '2.3.5' into ios-fastrenderers
* 2.3.5:
  [UWP] Fixes for usage of XF with .net native toolchain (#1024)
  [UWP] Make sure to update HitTestVisible when IsEnable changes (#1015)
  [Android] Dispose check before setting properties on Button (#1013)
  Add missing member variable to FormsApplicationActivity
  Fix NRE when background color of button set in FormsApplicationActivity (#1010)
  Fix border on android buttons  (#941)
  [iOS] ListView with UnevenRows and Cell Heights will no longer be slow to load (#994)
  Set the Id field for Android Views created by Forms #1004
  Fix build
  Fix possible crash on API 21+ at launch when using Holo theme and FormsApplicationActivity (#961)
  [Android] Remove the ". " on empty labels (Accessibility) on Fastrenderers (#915)
  Remove debug outputs (#1008)
  Add check for instance of UITableView (#885)
  [XamlC] fix release builds of Xaml Unit Tests
  Dispose check on ButtonRenderer (#975)
  [previewer] make sure we do not crash even if the previewer doesn't s… (#946)
  [XamlC] fix build
  Remove VisualElement finalizer (#918)
  [XamlC] process symbols if DebugType is set (#925)

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts added this to the 2.3.5 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment