Skip to content

Conversation

@JesseCol
Copy link
Contributor

@JesseCol JesseCol commented Feb 5, 2025

Description

Adds a ContentIsland page that uses Scene Node APIs to show a 3D model of a helmet.

Motivation and Context

This shows how apps can use ChildSiteLink to attach a ContentIsland in a Xaml app.

How Has This Been Tested?

  • Built and ran packaged
  • Built and ran unpackaged

Screenshots (if appropriate):

ContentIslandHelmet

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Copy link

@jeffstall jeffstall left a comment

Choose a reason for hiding this comment

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

Can we just check into the experimental branch now, and then merge that branch into main once WASDK 1.7.0 is GA?

@niels9001
Copy link
Collaborator

@JesseCol I've updated your PR to resolve major conflicts due to #1732. I've also tweaked the XAML a bit to remove unnecessary XAML and used a WrapPanel vs. the Grid so things are easier to see on a wide-screens.

@JesseCol
Copy link
Contributor Author

JesseCol commented Feb 7, 2025

Can we just check into the experimental branch now, and then merge that branch into main once WASDK 1.7.0 is GA?

Hi, there's no dedicated experimental branch for this repo (WinUI-Gallery). I've heard there's a new plan for WindowsAppSDK-Samples where we'll check in code that uses experimental APIs to main, and then conditionally compile that code. Maybe we can do something similar here.

@marcelwgn
Copy link
Contributor

Can we just check into the experimental branch now, and then merge that branch into main once WASDK 1.7.0 is GA?

Hi, there's no dedicated experimental branch for this repo (WinUI-Gallery). I've heard there's a new plan for WindowsAppSDK-Samples where we'll check in code that uses experimental APIs to main, and then conditionally compile that code. Maybe we can do something similar here.

For that, we essentially only need to check for WinUI 1.7 in the csproj and .cs file right?

@JesseCol
Copy link
Contributor Author

Can we just check into the experimental branch now, and then merge that branch into main once WASDK 1.7.0 is GA?

Hi, there's no dedicated experimental branch for this repo (WinUI-Gallery). I've heard there's a new plan for WindowsAppSDK-Samples where we'll check in code that uses experimental APIs to main, and then conditionally compile that code. Maybe we can do something similar here.

For that, we essentially only need to check for WinUI 1.7 in the csproj and .cs file right?

Well, the problem as I understand it is that:

  • 1.7 exp2 DOES have these APIs.
  • 1.7 preview1 does NOT have them.
  • 1.7 GA WILL have them (we expect).

I'm assuming we'll want to build Gallery with preview1 at some point, and if we did this would break.

Maybe we could query the MicrosoftWindowsAppSDKChannel MSBuild property, and only use these APIs when it's set to "experimental" or "stable" (and not preview), but that seems like it's getting kinda fiddly.

Adding @karkarl as well for more input.

@niels9001 niels9001 marked this pull request as draft February 16, 2025 12:17
@niels9001 niels9001 self-requested a review March 10, 2025 16:49
Copy link
Collaborator

@niels9001 niels9001 left a comment

Choose a reason for hiding this comment

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

Let's do a final check when 1.7 stable is available and then we can get this in!

@niels9001 niels9001 marked this pull request as ready for review March 19, 2025 08:59
@niels9001
Copy link
Collaborator

After upgrading to 1.7-stable:

image

@JesseCol
Copy link
Contributor Author

After upgrading to 1.7-stable:

image

Ah thanks -- yes, we changed this to be a ContentIsland prop getter.

Copy link
Contributor

@Zakariathr22 Zakariathr22 left a comment

Choose a reason for hiding this comment

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

Wouldn't it be better to include this in a ControlExample with code samples, as it was done in all the other samples?

@niels9001 niels9001 enabled auto-merge (squash) March 20, 2025 16:31
@niels9001
Copy link
Collaborator

/azp run

1 similar comment
@niels9001
Copy link
Collaborator

/azp run

@niels9001 niels9001 dismissed ionvasilian’s stale review March 23, 2025 12:06

Comments has been resolved.

@niels9001 niels9001 merged commit 1ceb1df into main Mar 23, 2025
2 checks passed
@niels9001 niels9001 deleted the user/jessecol/contentislandpage branch March 23, 2025 12:06
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.

7 participants