Skip to content

Commit d329ca7

Browse files
Copilotmattleibow
Copilot
and
mattleibow
committed
Address PR feedback: implement proper coercion pattern, fix handlers, and improve tests
Co-authored-by: mattleibow <1096616+mattleibow@users.noreply.github.com>
1 parent 35b693c commit d329ca7

File tree

18 files changed

+227
-89
lines changed

18 files changed

+227
-89
lines changed

‎src/Controls/src/Core/PublicAPI/net-android/PublicAPI.Unshipped.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
Microsoft.Maui.Controls.FlexLayout.CrossPlatformMeasure(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
44
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.get -> bool
55
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.set -> void
6+
Microsoft.Maui.Controls.RefreshView.RefreshIsRefreshEnabledProperty() -> void
67
override Microsoft.Maui.Controls.ContentPresenter.OnSizeAllocated(double width, double height) -> void
8+
*REMOVED*override Microsoft.Maui.Controls.RefreshView.IsEnabledCore.get -> bool
79
override Microsoft.Maui.Controls.ScrollView.OnSizeAllocated(double width, double height) -> void
810
override Microsoft.Maui.Controls.TemplatedView.OnSizeAllocated(double width, double height) -> void
911
~static readonly Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledProperty -> Microsoft.Maui.Controls.BindableProperty
@@ -20,3 +22,4 @@ virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDec
2022
~virtual Microsoft.Maui.Controls.CollectionSynchronizationCallback.Invoke(System.Collections.IEnumerable collection, object context, System.Action accessMethod, bool writeAccess) -> void
2123
~virtual Microsoft.Maui.Controls.Internals.EvaluateJavaScriptDelegate.Invoke(string script) -> System.Threading.Tasks.Task<string>
2224
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
25+
virtual Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledCore.get -> bool

‎src/Controls/src/Core/PublicAPI/net-tizen/PublicAPI.Unshipped.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
Microsoft.Maui.Controls.FlexLayout.CrossPlatformMeasure(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
44
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.get -> bool
55
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.set -> void
6+
Microsoft.Maui.Controls.RefreshView.RefreshIsRefreshEnabledProperty() -> void
67
override Microsoft.Maui.Controls.ContentPresenter.OnSizeAllocated(double width, double height) -> void
8+
*REMOVED*override Microsoft.Maui.Controls.RefreshView.IsEnabledCore.get -> bool
79
override Microsoft.Maui.Controls.ScrollView.OnSizeAllocated(double width, double height) -> void
810
override Microsoft.Maui.Controls.TemplatedView.OnSizeAllocated(double width, double height) -> void
911
~static readonly Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledProperty -> Microsoft.Maui.Controls.BindableProperty
@@ -20,3 +22,4 @@ virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDec
2022
~virtual Microsoft.Maui.Controls.CollectionSynchronizationCallback.Invoke(System.Collections.IEnumerable collection, object context, System.Action accessMethod, bool writeAccess) -> void
2123
~virtual Microsoft.Maui.Controls.Internals.EvaluateJavaScriptDelegate.Invoke(string script) -> System.Threading.Tasks.Task<string>
2224
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
25+
virtual Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledCore.get -> bool

‎src/Controls/src/Core/PublicAPI/net/PublicAPI.Unshipped.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
Microsoft.Maui.Controls.FlexLayout.CrossPlatformMeasure(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
44
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.get -> bool
55
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.set -> void
6+
Microsoft.Maui.Controls.RefreshView.RefreshIsRefreshEnabledProperty() -> void
67
override Microsoft.Maui.Controls.ContentPresenter.OnSizeAllocated(double width, double height) -> void
8+
*REMOVED*override Microsoft.Maui.Controls.RefreshView.IsEnabledCore.get -> bool
79
override Microsoft.Maui.Controls.ScrollView.OnSizeAllocated(double width, double height) -> void
810
override Microsoft.Maui.Controls.TemplatedView.OnSizeAllocated(double width, double height) -> void
911
~static readonly Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledProperty -> Microsoft.Maui.Controls.BindableProperty
@@ -20,3 +22,4 @@ virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDec
2022
~virtual Microsoft.Maui.Controls.CollectionSynchronizationCallback.Invoke(System.Collections.IEnumerable collection, object context, System.Action accessMethod, bool writeAccess) -> void
2123
~virtual Microsoft.Maui.Controls.Internals.EvaluateJavaScriptDelegate.Invoke(string script) -> System.Threading.Tasks.Task<string>
2224
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
25+
virtual Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledCore.get -> bool

‎src/Controls/src/Core/PublicAPI/netstandard/PublicAPI.Unshipped.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
Microsoft.Maui.Controls.FlexLayout.CrossPlatformMeasure(double widthConstraint, double heightConstraint) -> Microsoft.Maui.Graphics.Size
44
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.get -> bool
55
Microsoft.Maui.Controls.RefreshView.IsRefreshEnabled.set -> void
6+
Microsoft.Maui.Controls.RefreshView.RefreshIsRefreshEnabledProperty() -> void
67
override Microsoft.Maui.Controls.ContentPresenter.OnSizeAllocated(double width, double height) -> void
8+
*REMOVED*override Microsoft.Maui.Controls.RefreshView.IsEnabledCore.get -> bool
79
override Microsoft.Maui.Controls.ScrollView.OnSizeAllocated(double width, double height) -> void
810
override Microsoft.Maui.Controls.TemplatedView.OnSizeAllocated(double width, double height) -> void
911
~static readonly Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledProperty -> Microsoft.Maui.Controls.BindableProperty
@@ -20,3 +22,4 @@ virtual Microsoft.Maui.Controls.BindableProperty.CreateDefaultValueDelegate<TDec
2022
~virtual Microsoft.Maui.Controls.CollectionSynchronizationCallback.Invoke(System.Collections.IEnumerable collection, object context, System.Action accessMethod, bool writeAccess) -> void
2123
~virtual Microsoft.Maui.Controls.Internals.EvaluateJavaScriptDelegate.Invoke(string script) -> System.Threading.Tasks.Task<string>
2224
~virtual Microsoft.Maui.Controls.PropertyChangingEventHandler.Invoke(object sender, Microsoft.Maui.Controls.PropertyChangingEventArgs e) -> void
25+
virtual Microsoft.Maui.Controls.RefreshView.IsRefreshEnabledCore.get -> bool

‎src/Controls/src/Core/RefreshView/RefreshView.cs‎

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,51 @@ public Color RefreshColor
111111

112112
/// <summary>Bindable property for <see cref="IsRefreshEnabled"/>.</summary>
113113
public static readonly BindableProperty IsRefreshEnabledProperty =
114-
BindableProperty.Create(nameof(IsRefreshEnabled), typeof(bool), typeof(RefreshView), true);
114+
BindableProperty.Create(nameof(IsRefreshEnabled), typeof(bool), typeof(RefreshView), true,
115+
propertyChanged: OnIsRefreshEnabledPropertyChanged, coerceValue: CoerceIsRefreshEnabledProperty);
116+
117+
bool _isRefreshEnabledExplicit = (bool)IsRefreshEnabledProperty.DefaultValue;
118+
119+
static object CoerceIsRefreshEnabledProperty(BindableObject bindable, object value)
120+
{
121+
if (bindable is RefreshView refreshView)
122+
{
123+
refreshView._isRefreshEnabledExplicit = (bool)value;
124+
return refreshView.IsRefreshEnabledCore;
125+
}
126+
127+
return false;
128+
}
129+
130+
static void OnIsRefreshEnabledPropertyChanged(BindableObject bindable, object oldValue, object newValue)
131+
{
132+
var refreshView = (RefreshView)bindable;
133+
if (refreshView == null)
134+
return;
135+
136+
// If IsRefreshEnabled becomes false and we're refreshing, stop the refresh
137+
if (!(bool)newValue && refreshView.IsRefreshing)
138+
refreshView.IsRefreshing = false;
139+
}
140+
141+
/// <summary>
142+
/// This value represents the cumulative IsRefreshEnabled value.
143+
/// It considers both the explicitly set value and the command's CanExecute state.
144+
/// </summary>
145+
protected virtual bool IsRefreshEnabledCore
146+
{
147+
get
148+
{
149+
if (_isRefreshEnabledExplicit == false)
150+
{
151+
// If the explicitly set value is false, then nothing else matters
152+
return false;
153+
}
154+
155+
// Also check if the command can execute
156+
return CommandElement.GetCanExecute(this);
157+
}
158+
}
115159

116160
/// <summary>
117161
/// Gets or sets a value indicating whether the pull-to-refresh gesture is enabled.
@@ -123,6 +167,13 @@ public bool IsRefreshEnabled
123167
set { SetValue(IsRefreshEnabledProperty, value); }
124168
}
125169

170+
/// <summary>
171+
/// This method must always be called if some event occurs and the value of
172+
/// the <see cref="IsRefreshEnabledCore"/> property will change.
173+
/// </summary>
174+
protected void RefreshIsRefreshEnabledProperty() =>
175+
this.RefreshPropertyValue(IsRefreshEnabledProperty, _isRefreshEnabledExplicit);
176+
126177
/// <inheritdoc/>
127178
public IPlatformElementConfiguration<T, RefreshView> On<T>() where T : IConfigPlatform
128179
{
@@ -133,24 +184,22 @@ public IPlatformElementConfiguration<T, RefreshView> On<T>() where T : IConfigPl
133184

134185
object ICommandElement.CommandParameter => CommandParameter;
135186

136-
protected override bool IsEnabledCore => base.IsEnabledCore && CommandElement.GetCanExecute(this);
137-
138187
bool IRefreshView.IsRefreshEnabled => IsRefreshEnabled;
139188

140189
void ICommandElement.CanExecuteChanged(object sender, EventArgs e)
141190
{
142191
if (IsRefreshing)
143192
return;
144193

145-
RefreshIsEnabledProperty();
194+
RefreshIsRefreshEnabledProperty();
146195
}
147196

148197
protected override void OnPropertyChanged([CallerMemberName] string propertyName = null)
149198
{
150199
base.OnPropertyChanged(propertyName);
151200

152-
if ((IsEnabledProperty.PropertyName == propertyName && !IsEnabled) ||
153-
(IsRefreshEnabledProperty.PropertyName == propertyName && !IsRefreshEnabled))
201+
// When IsEnabled becomes false, stop any active refresh
202+
if (IsEnabledProperty.PropertyName == propertyName && !IsEnabled)
154203
{
155204
if (IsRefreshing)
156205
IsRefreshing = false;

‎src/Controls/tests/Core.UnitTests/RefreshViewTests.cs‎

Lines changed: 85 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@ public void CanExecuteDisablesRefreshView()
3434
{
3535
RefreshView refreshView = new RefreshView();
3636
refreshView.Command = new Command(() => { }, () => false);
37-
Assert.False(refreshView.IsEnabled);
37+
Assert.False(refreshView.IsRefreshEnabled);
3838
}
3939

4040
[Fact]
4141
public void CanExecuteEnablesRefreshView()
4242
{
4343
RefreshView refreshView = new RefreshView();
4444
refreshView.Command = new Command(() => { }, () => true);
45-
Assert.True(refreshView.IsEnabled);
45+
Assert.True(refreshView.IsRefreshEnabled);
4646
}
4747

4848
[Fact]
@@ -78,12 +78,12 @@ public void CanExecuteChangesEnabled()
7878

7979
canExecute = false;
8080
command.ChangeCanExecute();
81-
Assert.False(refreshView.IsEnabled);
81+
Assert.False(refreshView.IsRefreshEnabled);
8282

8383

8484
canExecute = true;
8585
command.ChangeCanExecute();
86-
Assert.True(refreshView.IsEnabled);
86+
Assert.True(refreshView.IsRefreshEnabled);
8787
}
8888

8989
[Fact]
@@ -96,11 +96,11 @@ public void CommandPropertyChangesEnabled()
9696
refreshView.CommandParameter = true;
9797
refreshView.Command = command;
9898

99-
Assert.True(refreshView.IsEnabled);
99+
Assert.True(refreshView.IsRefreshEnabled);
100100
refreshView.CommandParameter = false;
101-
Assert.False(refreshView.IsEnabled);
101+
Assert.False(refreshView.IsRefreshEnabled);
102102
refreshView.CommandParameter = true;
103-
Assert.True(refreshView.IsEnabled);
103+
Assert.True(refreshView.IsRefreshEnabled);
104104
}
105105

106106
[Fact]
@@ -111,11 +111,11 @@ public void RemovedCommandEnablesRefreshView()
111111
bool canExecute = true;
112112
var command = new Command(() => { }, () => false);
113113
refreshView.Command = command;
114-
Assert.False(refreshView.IsEnabled);
114+
Assert.False(refreshView.IsRefreshEnabled);
115115
refreshView.Command = null;
116-
Assert.True(refreshView.IsEnabled);
116+
Assert.True(refreshView.IsRefreshEnabled);
117117
refreshView.Command = command;
118-
Assert.False(refreshView.IsEnabled);
118+
Assert.False(refreshView.IsRefreshEnabled);
119119
}
120120

121121
[Fact]
@@ -192,17 +192,6 @@ public void IsRefreshEnabledPreventsIsRefreshingFromBeingSetToTrue()
192192
Assert.False(refreshView.IsRefreshing);
193193
}
194194

195-
[Fact]
196-
public void IsRefreshingCanBeSetToFalseWhenIsRefreshEnabledIsFalse()
197-
{
198-
RefreshView refreshView = new RefreshView();
199-
refreshView.IsRefreshing = true;
200-
Assert.True(refreshView.IsRefreshing);
201-
202-
refreshView.IsRefreshEnabled = false;
203-
Assert.False(refreshView.IsRefreshing); // Should be automatically cleared
204-
}
205-
206195
[Fact]
207196
public void SettingIsRefreshEnabledToFalseWhileRefreshingStopsRefresh()
208197
{
@@ -214,28 +203,20 @@ public void SettingIsRefreshEnabledToFalseWhileRefreshingStopsRefresh()
214203
Assert.False(refreshView.IsRefreshing);
215204
}
216205

217-
[Fact]
218-
public void BothIsEnabledAndIsRefreshEnabledMustBeTrueToAllowRefresh()
206+
[Theory]
207+
[InlineData(true, true, true, true)] // Both enabled - should allow refresh
208+
[InlineData(false, true, true, false)] // IsEnabled false - should prevent refresh
209+
[InlineData(true, false, true, false)] // IsRefreshEnabled false - should prevent refresh
210+
[InlineData(false, false, true, false)] // Both false - should prevent refresh
211+
public void RefreshBehaviorDependsOnIsEnabledAndIsRefreshEnabled(bool isEnabled, bool isRefreshEnabled, bool setRefreshing, bool expectedRefreshing)
219212
{
220213
RefreshView refreshView = new RefreshView();
221214

222-
// Both true - should allow refresh
223-
refreshView.IsEnabled = true;
224-
refreshView.IsRefreshEnabled = true;
225-
refreshView.IsRefreshing = true;
226-
Assert.True(refreshView.IsRefreshing);
215+
refreshView.IsEnabled = isEnabled;
216+
refreshView.IsRefreshEnabled = isRefreshEnabled;
217+
refreshView.IsRefreshing = setRefreshing;
227218

228-
// IsEnabled false - should prevent refresh
229-
refreshView.IsRefreshing = false;
230-
refreshView.IsEnabled = false;
231-
refreshView.IsRefreshing = true;
232-
Assert.False(refreshView.IsRefreshing);
233-
234-
// IsRefreshEnabled false - should prevent refresh
235-
refreshView.IsEnabled = true;
236-
refreshView.IsRefreshEnabled = false;
237-
refreshView.IsRefreshing = true;
238-
Assert.False(refreshView.IsRefreshing);
219+
Assert.Equal(expectedRefreshing, refreshView.IsRefreshing);
239220
}
240221

241222
[Fact]
@@ -260,5 +241,70 @@ public void IsRefreshEnabledWorksWithCommand()
260241
Assert.False(refreshView.IsRefreshing);
261242
Assert.False(commandExecuted);
262243
}
244+
245+
[Fact]
246+
public void IsRefreshEnabledRespectsCommandCanExecute()
247+
{
248+
RefreshView refreshView = new RefreshView();
249+
bool canExecute = true;
250+
bool commandExecuted = false;
251+
252+
refreshView.Command = new Command(() => commandExecuted = true, () => canExecute);
253+
254+
// Initially can execute and IsRefreshEnabled is true by default
255+
Assert.True(refreshView.IsRefreshEnabled);
256+
257+
// When command cannot execute, IsRefreshEnabled should be false
258+
canExecute = false;
259+
((Command)refreshView.Command).ChangeCanExecute();
260+
Assert.False(refreshView.IsRefreshEnabled);
261+
262+
// When command can execute again, IsRefreshEnabled should be true (if explicitly set)
263+
canExecute = true;
264+
((Command)refreshView.Command).ChangeCanExecute();
265+
Assert.True(refreshView.IsRefreshEnabled);
266+
}
267+
268+
[Fact]
269+
public void IsRefreshEnabledWithCommandCanExecuteFalseBlocksRefresh()
270+
{
271+
RefreshView refreshView = new RefreshView();
272+
bool canExecute = false;
273+
bool commandExecuted = false;
274+
275+
refreshView.Command = new Command(() => commandExecuted = true, () => canExecute);
276+
277+
// Even though IsRefreshEnabled is explicitly true, command cannot execute
278+
refreshView.IsRefreshEnabled = true;
279+
Assert.False(refreshView.IsRefreshEnabled); // Should be coerced to false
280+
281+
// Trying to refresh should fail
282+
refreshView.IsRefreshing = true;
283+
Assert.False(refreshView.IsRefreshing);
284+
Assert.False(commandExecuted);
285+
}
286+
287+
[Fact]
288+
public void CommandCanExecuteChangeClearsIsRefreshingWhenBecomesFalse()
289+
{
290+
RefreshView refreshView = new RefreshView();
291+
bool canExecute = true;
292+
bool commandExecuted = false;
293+
294+
refreshView.Command = new Command(() => commandExecuted = true, () => canExecute);
295+
296+
// Start refreshing
297+
refreshView.IsRefreshing = true;
298+
Assert.True(refreshView.IsRefreshing);
299+
Assert.True(commandExecuted);
300+
301+
// When command can no longer execute, refreshing should stop
302+
canExecute = false;
303+
((Command)refreshView.Command).ChangeCanExecute();
304+
305+
// The refresh should not be stopped by CanExecuteChanged when already refreshing
306+
// This matches the behavior in the CanExecuteChanged method
307+
Assert.True(refreshView.IsRefreshing);
308+
}
263309
}
264310
}

0 commit comments

Comments
 (0)