Skip to content

Commit 0e24e05

Browse files
author
Dilip Ojha
authored
Fix ItemsRepeater overwriting DataContext (#1740)
* fix itemsrepeater overwriting datacontext * added two tests
1 parent 7d36af3 commit 0e24e05

File tree

5 files changed

+197
-36
lines changed

5 files changed

+197
-36
lines changed

‎dev/Repeater/APITests/Common/DataAsElementElementFactory.cs‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License. See LICENSE in the project root for license information.
3+
4+
using System;
5+
using Windows.UI.Xaml.Controls;
6+
7+
namespace Windows.UI.Xaml.Tests.MUXControls.ApiTests.RepeaterTests.Common
8+
{
9+
using Microsoft.UI.Xaml.Controls;
10+
using ElementFactory = Microsoft.UI.Xaml.Controls.ElementFactory;
11+
12+
class DataAsElementElementFactory : ElementFactory
13+
{
14+
protected override UIElement GetElementCore(ElementFactoryGetArgs args)
15+
{
16+
return args.Data as UIElement;
17+
}
18+
19+
protected override void RecycleElementCore(ElementFactoryRecycleArgs args)
20+
{
21+
}
22+
}
23+
}

‎dev/Repeater/APITests/Common/ElementFromElementElementFactory.cs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
// Copyright (c) Microsoft Corporation. All rights reserved.
2+
// Licensed under the MIT License. See LICENSE in the project root for license information.
3+
4+
using System;
5+
using Windows.UI.Xaml.Controls;
6+
7+
namespace Windows.UI.Xaml.Tests.MUXControls.ApiTests.RepeaterTests.Common
8+
{
9+
using Microsoft.UI.Xaml.Controls;
10+
using ElementFactory = Microsoft.UI.Xaml.Controls.ElementFactory;
11+
12+
class ElementFromElementElementFactory : ElementFactory
13+
{
14+
protected override UIElement GetElementCore(ElementFactoryGetArgs args)
15+
{
16+
var button = new Button();
17+
button.Content = args.Data;
18+
return button;
19+
}
20+
21+
protected override void RecycleElementCore(ElementFactoryRecycleArgs args)
22+
{
23+
var container = args.Element as Button;
24+
container.Content = null;
25+
}
26+
}
27+
}

‎dev/Repeater/APITests/Repeater_APITests.projitems‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,11 @@
2828
<Compile Include="$(MSBuildThisFileDirectory)Common\SharedHelpers.cs" />
2929
<Compile Include="$(MSBuildThisFileDirectory)Common\TestsBase.cs" />
3030
<Compile Include="$(MSBuildThisFileDirectory)Common\WinRTCollection.cs" />
31+
<Compile Include="$(MSBuildThisFileDirectory)Common\DataAsElementElementFactory.cs" />
3132
<Compile Include="$(MSBuildThisFileDirectory)EffectiveViewportScrollerTests.cs" />
3233
<Compile Include="$(MSBuildThisFileDirectory)EffectiveViewportScrollViewerTests.cs" />
3334
<Compile Include="$(MSBuildThisFileDirectory)ElementAnimatorTests.cs" />
35+
<Compile Include="$(MSBuildThisFileDirectory)Common\ElementFromElementElementFactory.cs" />
3436
<Compile Include="$(MSBuildThisFileDirectory)FlowLayoutCollectionChangeTests.cs" />
3537
<Compile Include="$(MSBuildThisFileDirectory)FlowLayoutTests.cs" />
3638
<Compile Include="$(MSBuildThisFileDirectory)IndexPathTests.cs" />

‎dev/Repeater/APITests/ViewManagerTests.cs‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,78 @@ public void CanResetLayoutAfterUniqueIdReset()
586586
});
587587
}
588588

589+
[TestMethod]
590+
public void ValidateDataContextDoesNotGetOverwritten()
591+
{
592+
const string c_element1DataContext = "Element1_DataContext";
593+
594+
RunOnUIThread.Execute(() =>
595+
{
596+
var data = new List<Button>()
597+
{
598+
new Button()
599+
{
600+
Content = "Element1_Content",
601+
DataContext = c_element1DataContext
602+
}
603+
};
604+
605+
var elementFactory = new DataAsElementElementFactory();
606+
607+
var repeater = new ItemsRepeater() {
608+
ItemsSource = data,
609+
ItemTemplate = elementFactory
610+
};
611+
612+
Content = repeater;
613+
614+
Content.UpdateLayout();
615+
616+
// Verify that DataContext is still the same
617+
var firstElement = repeater.TryGetElement(0) as Button;
618+
var retrievedDataContextItem1 = firstElement.DataContext as string;
619+
Verify.IsTrue(retrievedDataContextItem1 == c_element1DataContext);
620+
621+
});
622+
}
623+
624+
[TestMethod]
625+
public void ValidateDataContextGetsPropagated()
626+
{
627+
const string c_element1DataContext = "Element1_DataContext";
628+
629+
RunOnUIThread.Execute(() =>
630+
{
631+
var data = new List<Button>()
632+
{
633+
new Button()
634+
{
635+
Content = "Element1_Content",
636+
DataContext = c_element1DataContext
637+
}
638+
};
639+
640+
var elementFactory = new ElementFromElementElementFactory();
641+
642+
var repeater = new ItemsRepeater()
643+
{
644+
ItemsSource = data,
645+
ItemTemplate = elementFactory
646+
};
647+
648+
Content = repeater;
649+
650+
Content.UpdateLayout();
651+
652+
// Verify that DataContext of data has propagated to the container
653+
var firstElement = repeater.TryGetElement(0) as Button;
654+
var retrievedDataContextItem1 = firstElement.DataContext as string;
655+
Verify.IsTrue(data[0] == firstElement.Content);
656+
Verify.IsTrue(retrievedDataContextItem1 == c_element1DataContext);
657+
658+
});
659+
}
660+
589661
// [TestMethod] Issue 1018
590662
public void ValidateFocusMoveOnElementCleared()
591663
{

‎dev/Repeater/ViewManager.cpp‎

Lines changed: 73 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -597,49 +597,69 @@ winrt::UIElement ViewManager::GetElementFromPinnedElements(int index)
597597
return element;
598598
}
599599

600+
// There are several cases handled here with respect to which element gets returned and when DataContext is modified.
601+
//
602+
// 1. If there is no ItemTemplate:
603+
// 1.1 If data is a UIElement -> the data is returned
604+
// 1.2 If data is not a UIElement -> a default DataTemplate is used to fetch element and DataContext is set to data**
605+
//
606+
// 2. If there is an ItemTemplate:
607+
// 2.1 If data is not a FrameworkElement -> Element is fetched from ElementFactory and DataContext is set to the data**
608+
// 2.2 If data is a FrameworkElement:
609+
// 2.2.1 If Element returned by the ElementFactory is the same as the data -> Element (a.k.a. data) is returned as is
610+
// 2.2.2 If Element returned by the ElementFactory is not the same as the data
611+
// -> Element that is fetched from the ElementFactory is returned and
612+
// DataContext is set to the data's DataContext (if it exists), otherwise it is set to the data itself**
613+
//
614+
// **data context is set only if no x:Bind was used. ie. No data template component on the root.
600615
winrt::UIElement ViewManager::GetElementFromElementFactory(int index)
601616
{
602617
// The view generator is the provider of last resort.
603-
auto data = m_owner->ItemsSourceView().GetAt(index);
604-
605-
auto itemTemplateFactory = m_owner->ItemTemplateShim();
618+
auto const data = m_owner->ItemsSourceView().GetAt(index);
606619

607-
winrt::UIElement element = nullptr;
608-
bool itemsSourceContainsElements = false;
609-
if (!itemTemplateFactory)
610-
{
611-
element = data.try_as<winrt::UIElement>();
612-
// No item template provided and ItemsSource contains objects derived from UIElement.
613-
// In this case, just use the data directly as elements.
614-
itemsSourceContainsElements = element != nullptr;
615-
}
616-
617-
if (!element)
620+
auto const element = [this, data, index, providedElementFactory = m_owner->ItemTemplateShim()]()
618621
{
619-
if (!itemTemplateFactory)
622+
if (!providedElementFactory)
620623
{
621-
// If no ItemTemplate was provided, use a default
622-
auto factory = winrt::XamlReader::Load(L"<DataTemplate xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation'><TextBlock Text='{Binding}'/></DataTemplate>").as<winrt::DataTemplate>();
623-
m_owner->ItemTemplate(factory);
624-
itemTemplateFactory = m_owner->ItemTemplateShim();
624+
if (auto const dataAsElement = data.try_as<winrt::UIElement>())
625+
{
626+
return dataAsElement;
627+
}
625628
}
626629

627-
if (!m_ElementFactoryGetArgs)
630+
auto const elementFactory = [this, providedElementFactory]()
628631
{
629-
// Create one.
630-
m_ElementFactoryGetArgs = tracker_ref<winrt::ElementFactoryGetArgs>(m_owner, *winrt::make_self<ElementFactoryGetArgs>());
631-
}
632+
if (!providedElementFactory)
633+
{
634+
// If no ItemTemplate was provided, use a default
635+
auto const factory = winrt::XamlReader::Load(L"<DataTemplate xmlns='http://schemas.microsoft.com/winfx/2006/xaml/presentation'><TextBlock Text='{Binding}'/></DataTemplate>").as<winrt::DataTemplate>();
636+
m_owner->ItemTemplate(factory);
637+
return m_owner->ItemTemplateShim();
638+
}
639+
return providedElementFactory;
640+
}();
641+
642+
auto const args = [this]()
643+
{
644+
if (!m_ElementFactoryGetArgs)
645+
{
646+
m_ElementFactoryGetArgs = tracker_ref<winrt::ElementFactoryGetArgs>(m_owner, *winrt::make_self<ElementFactoryGetArgs>());
647+
}
648+
return m_ElementFactoryGetArgs.get();
649+
}();
650+
651+
auto scopeGuard = gsl::finally([args]()
652+
{
653+
args.Data(nullptr);
654+
args.Parent(nullptr);
655+
});
632656

633-
auto args = m_ElementFactoryGetArgs.get();
634657
args.Data(data);
635658
args.Parent(*m_owner);
636659
args.as<ElementFactoryGetArgs>()->Index(index);
637660

638-
element = itemTemplateFactory.GetElement(args);
639-
640-
args.Data(nullptr);
641-
args.Parent(nullptr);
642-
}
661+
return elementFactory.GetElement(args);
662+
}();
643663

644664
auto virtInfo = ItemsRepeater::TryGetVirtualizationInfo(element);
645665
if (!virtInfo)
@@ -654,14 +674,13 @@ winrt::UIElement ViewManager::GetElementFromElementFactory(int index)
654674
REPEATER_TRACE_PERF(L"ElementRecycled");
655675
}
656676

657-
if (!itemsSourceContainsElements)
677+
if (data != element)
658678
{
659679
// Prepare the element
660680
// If we are phasing, run phase 0 before setting DataContext. If phase 0 is not
661681
// run before setting DataContext, when setting DataContext all the phases will be
662682
// run in the OnDataContextChanged handler in code generated by the xaml compiler (code-gen).
663-
auto extension = CachedVisualTreeHelpers::GetDataTemplateComponent(element);
664-
if (extension)
683+
if (auto extension = CachedVisualTreeHelpers::GetDataTemplateComponent(element))
665684
{
666685
// Clear out old data.
667686
extension.Recycle();
@@ -673,11 +692,29 @@ winrt::UIElement ViewManager::GetElementFromElementFactory(int index)
673692
// Update phase on virtInfo. Set data and templateComponent only if x:Phase was used.
674693
virtInfo->UpdatePhasingInfo(nextPhase, nextPhase > 0 ? data : nullptr, nextPhase > 0 ? extension : nullptr);
675694
}
676-
else
695+
else if(auto elementAsFE = element.try_as<winrt::FrameworkElement>())
677696
{
678697
// Set data context only if no x:Bind was used. ie. No data template component on the root.
679-
auto elementAsFE = element.try_as<winrt::FrameworkElement>();
680-
elementAsFE.DataContext(data);
698+
// If the passed in data is a UIElement and is different from the element returned by
699+
// the template factory then we need to propagate the DataContext.
700+
// Otherwise just set the DataContext on the element as the data.
701+
auto const elementDataContext = [this, data]()
702+
{
703+
if (auto const dataAsElement = data.try_as<winrt::FrameworkElement>())
704+
{
705+
if (auto const dataDataContext = dataAsElement.DataContext())
706+
{
707+
return dataDataContext;
708+
}
709+
}
710+
return data;
711+
}();
712+
713+
elementAsFE.DataContext(elementDataContext);
714+
}
715+
else
716+
{
717+
MUX_ASSERT(L"Element returned by factory is not a FrameworkElement!");
681718
}
682719
}
683720

@@ -704,7 +741,7 @@ winrt::UIElement ViewManager::GetElementFromElementFactory(int index)
704741

705742
repeater->OnElementPrepared(element, index);
706743

707-
if (!itemsSourceContainsElements)
744+
if (data != element)
708745
{
709746
m_phaser.PhaseElement(element, virtInfo);
710747
}

0 commit comments

Comments
 (0)