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

Add E2E Test App for Fabric #11407

Merged
merged 96 commits into from
Mar 30, 2023
Merged

Conversation

chiaramooney
Copy link
Contributor

@chiaramooney chiaramooney commented Mar 21, 2023

Description

Type of Change

  • New feature (non-breaking change which adds functionality)

Why

Adds E2E Test App which uses the new architecture.

#11422

What

Adds E2E Test App which uses the new architecture. App renders same content as original E2E test app.

Microsoft Reviewers: Open in CodeFlow

@jonthysell jonthysell added the Area: Fabric Support Facebook Fabric label Mar 21, 2023
yarn.lock Outdated Show resolved Hide resolved
packages/e2e-test-app-fabric/.gitignore Outdated Show resolved Hide resolved
packages/e2e-test-app-fabric/.gitignore Outdated Show resolved Hide resolved
Playground-Win32 is an unpackaged win32 app, and needs to use Microsoft.UI.Xaml from a prerelease,
to be able to carry WinUI in-app instead of using the Framework Package
-->
<WinUI2xVersion>2.7.0-prerelease.210913003</WinUI2xVersion>
Copy link
Contributor

Choose a reason for hiding this comment

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

@acoates-ms How should we be handling XAML in an explicitly Fabric comp app? Eventually we'll be able to host XAML with content islands. Does that mean all RNW apps will have to use pre-release (non-Framework) WinUI packages? Should we be ifndef-ing all WinUI code when Fabric is enabled so XAML isn't depended on by M.RN at all? Will we only support WinUI3 content islands?

Copy link
Contributor

Choose a reason for hiding this comment

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

Right now since no hosting of XAML is being done, it should all work with Win2 or 3. But when we get to hosting that would be 3 only. So we should probably make the test app winui3 from the start.

Alternatively, we could make the test app a win32 app with no XAML. -- Which I think aligns with the playground app.

Copy link
Contributor

Choose a reason for hiding this comment

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

As this is going to inform our app template, I would love to leave XAML out for now if possible. It would be nice to make XAML strictly opt-in (esp. if we end up not needing XAML for any core components).

Copy link
Contributor

Choose a reason for hiding this comment

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

While I'm okay with the package being named e2e-test-app-fabric while we work on it, is there any reason to name all of the files with -Farbic? Everything within the code still just refers to the app as RNTester (namespaces, strings) so maybe we just leave it all as RNTester? Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm flexible! In the original e2e-test-app, the name RNTesterApp is used. We could replicate that here, if we don't want to add the -Fabric specifier.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's a bunch of stuff in here we'll want to (eventually) move into a more re-usable place, maybe trying to follow the example of CoreApp, but it doesn't have to be in this particular initial PR.

chiaramooney and others added 9 commits March 22, 2023 13:21
Co-authored-by: Jon Thysell <jthysell@microsoft.com>
Co-authored-by: Jon Thysell <jthysell@microsoft.com>
Co-authored-by: Jon Thysell <jthysell@microsoft.com>
Co-authored-by: Jon Thysell <jthysell@microsoft.com>
Co-authored-by: Jon Thysell <jthysell@microsoft.com>
Co-authored-by: Jon Thysell <jthysell@microsoft.com>
Comment on lines +18 to +19
<!-- https://github.com/microsoft/react-native-windows/issues/8591 -->
<RunAutolinkCheck>false</RunAutolinkCheck>
Copy link
Contributor

Choose a reason for hiding this comment

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

we can leave it for now.

<PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
<ConfigurationType>Application</ConfigurationType>
<UseDebugLibraries>true</UseDebugLibraries>
<PlatformToolset>v143</PlatformToolset>
Copy link
Contributor

Choose a reason for hiding this comment

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

This might be and example of something we have to put in the Win32 props sheet that we didn't need for UWP apps.

@chiaramooney chiaramooney merged commit d5d19f5 into microsoft:main Mar 30, 2023
@chiaramooney chiaramooney deleted the cm-fabric-e2e branch March 30, 2023 22:40
jonthysell pushed a commit that referenced this pull request Jun 12, 2023
## Description

### Type of Change
- Code Edits

### Why
Adding some of the coding feedback that arose during #11407 into the Playground Composition source for consistency.

Resolves #11413
@jonthysell jonthysell added the New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric label Mar 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Fabric Support Facebook Fabric New Architecture Broad category for issues that apply to the RN "new" architecture of Turbo Modules + Fabric
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants