Skip to content

Conversation

@niels9001
Copy link
Collaborator

Porting over some changes from the UX exploration to the Sample Gallery.

UXgallery

Things to consider:

  • There's now a Title and Description textblock that reads the DisplayName and Description from the Metadata.. Not sure if that is the way to go (as duplicate information might be in the .md itself)?

@niels9001 niels9001 requested a review from michael-hawker June 16, 2022 16:02
@net-foundation-cla
Copy link

net-foundation-cla bot commented Jun 16, 2022

CLA assistant check
All CLA requirements met.

@michael-hawker
Copy link
Member

@niels9001 added comment for at least one thing causing the build to fail (unsupported property from uno, so just needs a guard prefix). Also you'll need to rebase the branch to get the header updates from main.

@niels9001 niels9001 force-pushed the niels9001/sample-app-UX branch from e940e4f to 5001319 Compare June 16, 2022 18:22
@michael-hawker
Copy link
Member

@niels9001 the muxc:BackdropMaterial.ApplyToRootOrPageBackground I think is both for WinUI 2 and 3 right?

"D:\a\Labs-Windows\Labs-Windows\template\lab\ProjectTemplate.sln" (default target) (1:2) ->
"D:\a\Labs-Windows\Labs-Windows\template\lab\samples\ProjectTemplate.WinAppSdk\ProjectTemplate.WinAppSdk.csproj" (default target) (6:6) ->
(MarkupCompilePass2 target) -> 
  D:\a\Labs-Windows\Labs-Windows\common\CommunityToolkit.Labs.Shared\NavigationPage.xaml(8,7): XamlCompiler error WMC0010: Unknown attachable member 'BackdropMaterial.ApplyToRootOrPageBackground' on element 'Page' [D:\a\Labs-Windows\Labs-Windows\template\lab\samples\ProjectTemplate.WinAppSdk\ProjectTemplate.WinAppSdk.csproj]
  D:\a\Labs-Windows\Labs-Windows\common\CommunityToolkit.Labs.Shared\TabbedPage.xaml(11,7): XamlCompiler error WMC0010: Unknown attachable member 'BackdropMaterial.ApplyToRootOrPageBackground' on element 'Page' [D:\a\Labs-Windows\Labs-Windows\template\lab\samples\ProjectTemplate.WinAppSdk\ProjectTemplate.WinAppSdk.csproj]

Though not supported on Uno. I know we use win: for anything in UWP.

@jeromelaban is there another special one for stuff that's specific to WinUI 2/3 from muxc? That's not called out in the doc specifically. Or could/would we use an IsType/PropertyPresent conditional, would that work? https://docs.microsoft.com/en-us/windows/uwp/debug-test-perf/conditional-xaml

@niels9001
Copy link
Collaborator Author

niels9001 commented Jun 16, 2022

@michael-hawker Actually, I think with WinUI 3 this API is not supported and requires code behind (and will only work on W11, not on W10..

Shall I leave it out for now? I guess in the future we need some sort of helper class that uses the right API when on WAS vs. UWP.

@niels9001 niels9001 requested a review from michael-hawker June 16, 2022 21:28
@michael-hawker
Copy link
Member

Thanks @niels9001, I did just follow-up on this, the approach with WinUI 3 is all different for now still yes: MicrosoftDocs/winui-api#121 (comment)

Let's leave the Mica out for now as you've done and we can evaluate an approach for that later either with a polyfill or helper or something else.

@michael-hawker
Copy link
Member

Tried out on the web in Codespaces and noticed some issues:

image

Looks like the metadata isn't getting bound for some reason in the header? This is probably because of unoplatform/uno#2872 - So just switch the bindings to OneWay vs. OneTime you have in the header, and I think that'll resolve it for now.

Also, the C# source code is centered compared to the XAML (so switching between the C#/XAML tabs is causing the text to jump around). Not sure what that may be. The only difference is that it's in an Expander now, eh? So it may be the HorizontalContentAlignment somewhere?

@niels9001
Copy link
Collaborator Author

Tried out on the web in Codespaces and noticed some issues:

image

Looks like the metadata isn't getting bound for some reason in the header? This is probably because of unoplatform/uno#2872 - So just switch the bindings to OneWay vs. OneTime you have in the header, and I think that'll resolve it for now.

Also, the C# source code is centered compared to the XAML (so switching between the C#/XAML tabs is causing the text to jump around). Not sure what that may be. The only difference is that it's in an Expander now, eh? So it may be the HorizontalContentAlignment somewhere?

Fixed! Metadata now correctly loading and the Pivot not stretches, so its content is now left aligned.

There are some minor visual glitches (e.g. not all icons are correctly loading and seem to occosianlly load MDL2 instead of Fluent Icons. Also the ComboBox does not default to the first item (SelectedIndex=0, but maybe small things we can fix later on?

@michael-hawker
Copy link
Member

@niels9001 looking good in Codespaces now (outside of Uno issue unoplatform/uno#4686)

However, I noticed in the TabbedPage view when running experiments that the source code expander isn't visible:

image

(I like that it fills up the whole app area by default, but think we need a scrollviewer or something around it. (This was just running the UWP head from the CanvasLayout.sln.)

You can see it becomes a bit of an issue when you shrink the app window size too:

image

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointed out spots for fixing the issues with the TabbedPage experience. Let me know your thoughts and what you want to tackle now vs. opening an issue for (build conditional to hide header type thing).

Also, don't forget to rebase off main as all the global usings are resolved there.

Text="See the docs" />
</StackPanel>
</Button>
<Button Grid.Column="1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seemed clipped on the right-hand side?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not seeing any clipping on my side? (Just the icon acts weird).

WASM:
image

UWP:
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Talking about the GitHub button here:
image

@michael-hawker
Copy link
Member

Also should the samples themselves be left-aligned vs. centered?

@niels9001
Copy link
Collaborator Author

Also should the samples themselves be left-aligned vs. centered?

Yeah, good suggestion - looks a bit cleaner. Left aligned now!

@niels9001 niels9001 force-pushed the niels9001/sample-app-UX branch from 3be691a to d3c6765 Compare June 22, 2022 12:57
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! Will merge now and we have a couple of small things to continue to improve and work on later, for instance #169.

@michael-hawker michael-hawker merged commit 107cc4b into main Jun 22, 2022
@delete-merged-branch delete-merged-branch bot deleted the niels9001/sample-app-UX branch June 22, 2022 20:13
Martin1994 pushed a commit to Martin1994/Labs-Windows that referenced this pull request Sep 2, 2023
* Overall layout, adding styles

* Add resizing logic and sample combobox

* Layout changes to documentrenderer

* Item rename

* Fix responsiveness

* Margin fix

* XAML formatting

* XAML formatting

* Catch Uno exception

* Updated Heading

* XAML formatting

* Fix

* Removed mica

* XAML formatting

* XAML formatting

* XAML formatting

* XAML formatting

* Feedback

* Responsize SampleOptions layout

* UI fixes for Uno

* Add margin to TabbedPage

* Remove redundant code

* Add SubtleButtonStyle

* XAML formatting

* Added visual state and left aligned samples

* Added ScrollViewer for samples on tabbed pages

* Removed brushtransition

* Uno fix
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