Skip to content
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

feat: ResponsiveView #912

Merged
merged 23 commits into from
Dec 4, 2023
Merged

feat: ResponsiveView #912

merged 23 commits into from
Dec 4, 2023

Conversation

eriklimakc
Copy link
Contributor

GitHub Issue (If applicable): #

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

To Discuss:

1. How we handle Template Content

What should be done when, for example, Narrow VisualState is triggered but NarrowContent is null? Should the DefaultContent be displayed instead? What would be the order for other states like Wide/ExtraWide?

switch (visualState)
{
case "ExtraNarrowSize":
contentToSet = ExtraNarrowContent ?? NarrowContent ?? DefaultContent ?? WideContent ?? ExtraWideContent;
break;
case "NarrowSize":
contentToSet = NarrowContent ?? ExtraNarrowContent ?? DefaultContent ?? WideContent ?? ExtraWideContent;
break;
case "DefaultSize":
contentToSet = DefaultContent ?? NarrowContent ?? WideContent ?? ExtraWideContent ?? ExtraNarrowContent;
break;
case "WideSize":
contentToSet = WideContent ?? DefaultContent ?? ExtraWideContent ?? NarrowContent ?? ExtraNarrowContent;
break;
case "ExtraWideSize":
contentToSet = ExtraWideContent ?? WideContent ?? DefaultContent ?? NarrowContent ?? ExtraNarrowContent;
break;
}

2. How to make the responsive settings global and overridable?

2.1 What should be the default setting for ExtraNarrow, Narrow, Default, etc... ?

<ResourceDictionary.ThemeDictionaries>
<ResourceDictionary x:Key="Default">
<x:Double x:Key="ResponsiveViewExtraNarrow">150</x:Double>
<x:Double x:Key="ResponsiveViewNarrow">350</x:Double>
<x:Double x:Key="ResponsiveViewDefault">800</x:Double>
<x:Double x:Key="ResponsiveViewWide">1200</x:Double>
<x:Double x:Key="ResponsiveViewExtraWide">1500</x:Double>
</ResourceDictionary>
<ResourceDictionary x:Key="Light">
<x:Double x:Key="ResponsiveViewExtraNarrow">150</x:Double>
<x:Double x:Key="ResponsiveViewNarrow">350</x:Double>
<x:Double x:Key="ResponsiveViewDefault">800</x:Double>
<x:Double x:Key="ResponsiveViewWide">1200</x:Double>
<x:Double x:Key="ResponsiveViewExtraWide">1500</x:Double>
</ResourceDictionary>
</ResourceDictionary.ThemeDictionaries>

3. Properties naming

Would it make more sense if the we named them, for example, ExtraNarrowTemplate instead of ExtraNarrowContent, or ExtraNarrowTemplateContent?

cc @Xiaoy312 @kazo0

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

1 similar comment
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

@eriklimakc eriklimakc marked this pull request as ready for review November 23, 2023 16:38
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

2 similar comments
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

@eriklimakc
Copy link
Contributor Author

@kazo0 and @Xiaoy312 , please note that my latest changes include:

#if WINDOWS || WINDOWS_UWP
	Content = dataTemplate?.LoadContent() as UIElement;
#else
        ContentTemplate = dataTemplate;
#endif

The reason behind this is that using ContentTemplate on non-windows platforms gives us a better experience when resizing the screen, see below:

ContentTemplate

ContentTemplate

Content

Content

* Using ContentTemplate on Windows doesn't work.

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

1 similar comment
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

@Xiaoy312
Copy link
Contributor

Xiaoy312 commented Nov 28, 2023

It shouldnt flicker this much on windows, with the Content. You need to cache the last resolved content-template or winning size, and compare this value on size-changed, and only update when the new ones are different than previous.
do not compare the result of ContentTemplate.Load() that is guaranteed to be different.
edit: and just to make the code simpler, you can drop the #if conditional, and to just cache the resolved content-template (in a separate field) and set the content with .Load() for any target.

Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

1 similar comment
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

doc/controls/ResponsiveView.md Outdated Show resolved Hide resolved
doc/controls/ResponsiveView.md Outdated Show resolved Hide resolved
doc/controls/ResponsiveView.md Show resolved Hide resolved
doc/controls/ResponsiveView.md Show resolved Hide resolved
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

1 similar comment
Copy link

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

Copy link

github-actions bot commented Dec 1, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

Copy link

github-actions bot commented Dec 4, 2023

Azure Static Web Apps: Your stage site is ready! Visit it here: https://polite-field-01aa64f0f-912.eastus2.azurestaticapps.net

@kazo0 kazo0 enabled auto-merge (squash) December 4, 2023 14:18
@kazo0 kazo0 merged commit 838a07a into main Dec 4, 2023
24 checks passed
@kazo0 kazo0 deleted the dev/erli/724-ResponsiveView branch December 4, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants