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
Don't worry about the size of the text #21628
Conversation
src/Controls/src/Core/Platform/Android/Extensions/ButtonExtensions.cs
Outdated
Show resolved
Hide resolved
// Because this resolves from a task we should validate that the | ||
// handler hasn't been disconnected | ||
if (handler.IsConnected()) | ||
handler.UpdateValue(nameof(IImage.IsAnimationPlaying)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was occasionally causing a crash in our device tests.
Also, I don't think we ever should have used ContinueWith here, I don't think ContinueWith
will resolve on the synchronization context. I'm pretty sure the code inside ContinueWith
will be on a task pool thread unless you pass TaskScheduler.FromCurrentSynchronizationContext()
src/Controls/src/Core/Platform/Android/Extensions/ButtonExtensions.cs
Outdated
Show resolved
Hide resolved
src/Controls/src/Core/Platform/Android/Extensions/ButtonExtensions.cs
Outdated
Show resolved
Hide resolved
05078ba
to
32698e2
Compare
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
The code looks good. It all seems reasonable and flows logically. I have not yet tested the actual code on a device/emulator. We probably need to throw in a UI test that updates the location of the image or shows/hides the image. I recall there were "update" issues as well. I think the current UI tests are just testing initial states. |
Failing tests unrelated |
Description of Change
After internal conversations and for the purposes of staying consistent with SR3 we're going to take the route of always letting the image take priority over the text for placement. There's not really a best way to determine the ratio to scale the image to when you have really long text. So, when the user hits a scenarios where the image is just too big for a constrained button it's the users responsibility to resize the physical image themselves. This is ultimately better for performance as well.