Merge pull request #404 from TimLariviere/fix-automationid-reuse

Added check to prevent control reuse with different AutomationId
fabulous-dev · May 11, 2019 · 7bdef72 · 7bdef72
2 parents 2f2f748 + 36d677e
commit 7bdef72
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 55 deletions.
diff --git a/.paket/Paket.Restore.targets b/.paket/Paket.Restore.targets
@@ -105,8 +105,8 @@
       <PaketRestoreRequired Condition=" '$(PaketRestoreLockFileHash)' == '' ">true</PaketRestoreRequired>
     </PropertyGroup>
 
-	<!--
-		This value should match the version in the props generated by paket
+	<!-- 
+		This value should match the version in the props generated by paket 
 		If they differ, this means we need to do a restore in order to ensure correct dependencies
 	-->
     <PropertyGroup Condition="'$(PaketPropsVersion)' != '5.185.3' ">
@@ -228,7 +228,6 @@
   </Target>
 
   <Target Name="PaketOverrideNuspec" AfterTargets="GenerateNuspec" Condition="('$(IsPackable)' == '' Or '$(IsPackable)' == 'true') And Exists('$(MSBuildProjectDirectory)/obj/$(MSBuildProjectFile).references')" >
-
     <ItemGroup>
       <_NuspecFilesNewLocation Include="$(BaseIntermediateOutputPath)$(Configuration)\*.nuspec"/>
       <MSBuildMajorVersion Include="$(DetectedMSBuildVersion.Replace(`-`, `.`).Split(`.`)[0])" />
@@ -238,12 +237,10 @@
     <PropertyGroup>
       <PaketProjectFile>$(MSBuildProjectDirectory)/$(MSBuildProjectFile)</PaketProjectFile>
       <ContinuePackingAfterGeneratingNuspec>true</ContinuePackingAfterGeneratingNuspec>
-      <UseMSBuild16_0_Pack>false</UseMSBuild16_0_Pack>
-      <UseMSBuild16_0_Pack Condition=" '@(MSBuildMajorVersion)' >= '16' ">true</UseMSBuild16_0_Pack>
       <UseMSBuild15_9_Pack>false</UseMSBuild15_9_Pack>
-      <UseMSBuild15_9_Pack Condition=" '@(MSBuildMajorVersion)' == '15' AND '@(MSBuildMinorVersion)' > '8' ">true</UseMSBuild15_9_Pack>
+      <UseMSBuild15_9_Pack Condition=" '@(MSBuildMajorVersion)' > '15' OR ('@(MSBuildMajorVersion)' == '15' AND '@(MSBuildMinorVersion)' > '8') ">true</UseMSBuild15_9_Pack>
       <UseMSBuild15_8_Pack>false</UseMSBuild15_8_Pack>
-      <UseMSBuild15_8_Pack Condition=" '$(NuGetToolVersion)' != '4.0.0' AND (! $(UseMSBuild15_9_Pack)) AND (! $(UseMSBuild16_0_Pack)) ">true</UseMSBuild15_8_Pack>
+      <UseMSBuild15_8_Pack Condition=" '$(NuGetToolVersion)' != '4.0.0' AND (! $(UseMSBuild15_9_Pack)) ">true</UseMSBuild15_8_Pack>
       <UseNuGet4_Pack>false</UseNuGet4_Pack>
       <UseNuGet4_Pack Condition=" (! $(UseMSBuild15_8_Pack)) AND (! $(UseMSBuild15_9_Pack)) ">true</UseNuGet4_Pack>
       <AdjustedNuspecOutputPath>$(BaseIntermediateOutputPath)$(Configuration)</AdjustedNuspecOutputPath>
@@ -261,53 +258,6 @@
     </ConvertToAbsolutePath>
 
     <!-- Call Pack -->
-    <PackTask Condition="$(UseMSBuild16_0_Pack)"
-              PackItem="$(PackProjectInputFile)"
-              PackageFiles="@(_PackageFiles)"
-              PackageFilesToExclude="@(_PackageFilesToExclude)"
-              PackageVersion="$(PackageVersion)"
-              PackageId="$(PackageId)"
-              Title="$(Title)"
-              Authors="$(Authors)"
-              Description="$(Description)"
-              Copyright="$(Copyright)"
-              RequireLicenseAcceptance="$(PackageRequireLicenseAcceptance)"
-              LicenseUrl="$(PackageLicenseUrl)"
-              ProjectUrl="$(PackageProjectUrl)"
-              IconUrl="$(PackageIconUrl)"
-              ReleaseNotes="$(PackageReleaseNotes)"
-              Tags="$(PackageTags)"
-              DevelopmentDependency="$(DevelopmentDependency)"
-              BuildOutputInPackage="@(_BuildOutputInPackage)"
-              TargetPathsToSymbols="@(_TargetPathsToSymbols)"
-              SymbolPackageFormat="symbols.nupkg"
-              TargetFrameworks="@(_TargetFrameworks)"
-              AssemblyName="$(AssemblyName)"
-              PackageOutputPath="$(PackageOutputAbsolutePath)"
-              IncludeSymbols="$(IncludeSymbols)"
-              IncludeSource="$(IncludeSource)"
-              PackageTypes="$(PackageType)"
-              IsTool="$(IsTool)"
-              RepositoryUrl="$(RepositoryUrl)"
-              RepositoryType="$(RepositoryType)"
-              SourceFiles="@(_SourceFiles->Distinct())"
-              NoPackageAnalysis="$(NoPackageAnalysis)"
-              MinClientVersion="$(MinClientVersion)"
-              Serviceable="$(Serviceable)"
-              FrameworkAssemblyReferences="@(_FrameworkAssemblyReferences)"
-              ContinuePackingAfterGeneratingNuspec="$(ContinuePackingAfterGeneratingNuspec)"
-              NuspecOutputPath="$(AdjustedNuspecOutputPath)"
-              IncludeBuildOutput="$(IncludeBuildOutput)"
-              BuildOutputFolders="$(BuildOutputTargetFolder)"
-              ContentTargetFolders="$(ContentTargetFolders)"
-              RestoreOutputPath="$(RestoreOutputAbsolutePath)"
-              NuspecFile="$(NuspecFileAbsolutePath)"
-              NuspecBasePath="$(NuspecBasePath)"
-              NuspecProperties="$(NuspecProperties)"
-              PackageLicenseFile="$(PackageLicenseFile)"
-              PackageLicenseExpression="$(PackageLicenseExpression)"
-              PackageLicenseExpressionVersion="$(PackageLicenseExpressionVersion)" />
-
     <PackTask Condition="$(UseMSBuild15_9_Pack)"
               PackItem="$(PackProjectInputFile)"
               PackageFiles="@(_PackageFiles)"

diff --git a/src/Fabulous.Core/ViewConverters.fs b/src/Fabulous.Core/ViewConverters.fs
@@ -219,7 +219,7 @@ module Converters =
 
     /// Checks whether an underlying control can be reused given the previous and new view elements
     let rec canReuseChild (prevChild:ViewElement) (newChild:ViewElement) =
-        if prevChild.TargetType = newChild.TargetType then
+        if prevChild.TargetType = newChild.TargetType && canReuseAutomationId prevChild newChild then
             if newChild.TargetType.IsAssignableFrom(typeof<NavigationPage>) then
                 canReuseNavigationPage prevChild newChild
             else
@@ -239,6 +239,16 @@ module Converters =
         | ValueSome prevPages, ValueSome newPages -> (prevPages, newPages) ||> Seq.forall2 canReuseChild
         | _, _ -> true
 
+    /// Checks whether the control can be reused given the previous and the new AutomationId.
+    /// Xamarin.Forms can't change an already setted AutomationId
+    and internal canReuseAutomationId (prevChild: ViewElement) (newChild: ViewElement) =
+        let prevAutomationId = prevChild.TryGetAttribute<string>("AutomationId")
+        let newAutomationId = newChild.TryGetAttribute<string>("AutomationId")
+
+        match prevAutomationId with
+        | ValueSome _ when prevAutomationId <> newAutomationId -> false
+        | _ -> true
+
     /// Update a control given the previous and new view elements
     let inline updateChild (prevChild:ViewElement) (newChild:ViewElement) targetChild = 
         newChild.UpdateIncremental(prevChild, targetChild)