microsoft / react-native-windows

A framework for building native Windows apps with React.
https://microsoft.github.io/react-native-windows/
Other
16.39k stars 1.14k forks source link

[Fabric] Implement onDismiss for Modal and remove titlebar #14126

Open TatianaKapos opened 6 days ago

TatianaKapos commented 6 days ago

Description

Implements onDismiss for Modal and removed the titlebar (Both Flyout/Popup/Modal don't have their own titlebar so by default Modal shouldn't have one either but it's easy to add it back in if needed! My guess is that the titlebar should be a windows-specific property).

This also means you no longer get the visual indicator that Modal needs to be dealt with before closing but Flyout/Popup also don't give a visual indicator.

Type of Change

Why

Resolves #13762 Resolves #14067

What

onDismiss doesn't work if you just implement the native-side because the Fabric-UI Manager destroys the Modal before the event can be fired. This PR enables the JavaScript portion of onDismiss for Modal that upstream implemented (see https://github.com/facebook/react-native/pull/42601)

Screenshots

Playground-composition_bAJKM2a6ik

Testing

tested locally

Changelog

yes

Implements onDismiss for Modal and removes titlebar

Microsoft Reviewers: Open in CodeFlow
jonthysell commented 3 days ago

We should probably (at least) have an internal m_showTitleBar member bool, maybe default to false (if that's the default behavior we want), and use that variable to control whether or not there's a title bar. Then maybe you could have whatever new title bar property you create (to lets users change it) be in a future PR.