Fix for Spinner not disappearing in RefreshView #16997
Conversation
…for spinner to disappear once IsRefreshing is set to false.
|
/azp run |
|
Azure Pipelines successfully started running 2 pipeline(s). |
dustin-wojciechowski
added
i/regression
ghost
added
the
legacy-area-controls
|
Regarding the spinner persisting locally, the issue still persists. To repro it, you have to drag down to start a refresh, and then (while still holding/tapping), drag up. The icon will remain stuck there. You can repro using the Controls.Sample app
|
|
Related #16654 |
| @@ -62,7 +62,7 @@ static object OnIsRefreshingPropertyCoerced(BindableObject bindable, object valu | |||
| public bool IsRefreshing | |||
| { | |||
| get { return (bool)GetValue(IsRefreshingProperty); } | |||
| set { SetValue(IsRefreshingProperty, value); } | |||
| set { SetValue(IsRefreshingProperty, value, SetterSpecificity.FromHandler); } | |||
There was a problem hiding this comment.
RefreshView isn't a Handler, so is this correct? @PureWeen does this actually need to go somewhere else?
There was a problem hiding this comment.
See Shane's comment here: #16997 (comment)
|
@jknaudt21 Thanks for reproducing that! I was able to get the same result with your video. I do think that is related to a different issue, as described in #8926. This one at least will fix the issue of the spinner lingering past the refresh period. |
| @@ -62,7 +62,7 @@ static object OnIsRefreshingPropertyCoerced(BindableObject bindable, object valu | |||
| public bool IsRefreshing | |||
| { | |||
| get { return (bool)GetValue(IsRefreshingProperty); } | |||
| set { SetValue(IsRefreshingProperty, value); } | |||
| set { SetValue(IsRefreshingProperty, value, SetterSpecificity.FromHandler); } | |||
There was a problem hiding this comment.
There are a few scenarios of this that we are currently fixing.
Use an explicit interface to use a different SetterSpecificity for values coming from the handler.
There was a problem hiding this comment.
this is wrong. the Handler should set the BP directly, not the c# property, with the right specificity
…dicate value coming from handler.
…o changed Refreshview.IsRefreshing's setter to use SetterSpecificity.FromBinding (This caused issue when hitting toggle button in samples app)
|
@PureWeen I did still have to set the SetterSpecificity to "FromBinding" in the setter of RefreshView.IsRefreshing to get this to work correctly. Let me know if that is the correct approach, still trying to understand SetterSpecificity better. |
Eilon
removed
the
legacy-area-controls
Description of Change
After some investigation, I found the PR where the spinner in Refresh Views would not disappear after IsRefreshing was set to false. After looking at the PR, I believe that the IsRefreshing property isn't utilizing the correct SetValue method.
Before (fast forward with scrubber to see the spinner, otherwise webm player won't show it):
before.webm
After:
after.webm
There still seems to be a case where the spinner's outline remains but the inner animation disappears, and it predates the Specificity PR by almost a year (yet was fixed at some later point, apparently). I haven't been able to reproduce that particular issue, and want to test some more, but figure I'd get some feedback on this one.
Issues Fixed
Fixes #16910