Use a prototype TextBlock to resolve theme resources on Windows #703

Merged
merged 3 commits into from Jan 26, 2017

Conversation

Projects
None yet
4 participants
@hartez
Member

hartez commented Jan 23, 2017

Description of Change

On the Windows platforms, the device styles were being retrieved and copied into the Forms resource dictionary by walking up the style tree and retrieving the Setter values. However, this fails when the walk up the style tree encounters a Setter whose value is a binding expression (e.g., to ThemeResource). Because of this, most of the built-in device styles do not render at all on the Windows platforms.

Rather than walk the tree, this change creates a TextBlock, applies the style to it, and retrieves the resolved values for use in the Forms resource dictionary. This allows it to retrieve values which come from ThemeResource bindings. The rendered device styles now match more closely with the values in WinRT apps created in Blend:

Windows Phone 8.1 Styles before
Windows Phone 8.1 Styles from Blend
Windows Phone 8.1 Styles after

Windows 8.1 Styles Before
Windows 8.1 Styles from Blend
Windows 8.1 Styles After

This also applies to UWP, but I got tired of taking screenshots.

Bugs Fixed

API Changes

  • None

Behavioral Changes

  • Anyone who previously relied on the incorrect device styles would need to update their code (though the updates would mostly consist of simply removing the device style).

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Consolidate commits as makes sense
- }
-
- var ancestorStyles = new List<Windows.UI.Xaml.Style> ();
+ readonly TextBlock _prototype = new TextBlock();

This comment has been minimized.

@samhouts

samhouts Jan 23, 2017

Member

Should we put this nearer the top? Seems weird here in the middle.

@samhouts

samhouts Jan 23, 2017

Member

Should we put this nearer the top? Seems weird here in the middle.

This comment has been minimized.

@hartez

hartez Jan 23, 2017

Member

Yes. Done.

@hartez

hartez Jan 23, 2017

Member

Yes. Done.

return formsStyle;
}
- static FontAttributes ToAttributes(ushort uweight)
+ readonly TextBlock _prototype = new TextBlock();

This comment has been minimized.

@samhouts

samhouts Jan 23, 2017

Member

Same question...should this go near the top of the file?

@samhouts

samhouts Jan 23, 2017

Member

Same question...should this go near the top of the file?

+namespace Xamarin.Forms.Controls.Issues
+{
+ [Preserve(AllMembers = true)]
+ [Issue(IssueTracker.Bugzilla, 43783, "[WP8.1] Most Device Styles do not render correctly in Windows Phone 8.1 (RT) applications", PlatformAffected.WinPhone)]

This comment has been minimized.

@samhouts

samhouts Jan 23, 2017

Member

Looks like these code changes also affect non-phones, right?

@samhouts

samhouts Jan 23, 2017

Member

Looks like these code changes also affect non-phones, right?

This comment has been minimized.

@hartez

hartez Jan 23, 2017

Member

Yes. The issue exists on all WinRT/UWP platforms.

@hartez

hartez Jan 23, 2017

Member

Yes. The issue exists on all WinRT/UWP platforms.

hartez added some commits Jan 23, 2017

@hartez hartez merged commit 520ff4a into master Jan 26, 2017

@hartez hartez deleted the fix-bugzilla43783 branch May 16, 2017

@samhouts samhouts added D-15.4 and removed cla-not-required labels Oct 10, 2017

@samhouts samhouts added this to the 2.3.5 milestone Jun 27, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment