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 very basic box-shadow support #14028

Merged
merged 2 commits into from
Oct 24, 2024
Merged

Conversation

acoates-ms
Copy link
Contributor

@acoates-ms acoates-ms commented Oct 23, 2024

Description

This hooks up support for a single box-shadow. If multiple shadows are specified, we ignore all but the first shadow. If box-shadow is provided, then its value is used instead of the legacy shadow properties.

This is a partial fix for #13944

There is another issue where shadows are not correctly showing currently on Views with a border radius. (#14027). That also affects the existing shadow properties.

Microsoft Reviewers: Open in CodeFlow

@acoates-ms acoates-ms requested a review from a team as a code owner October 23, 2024 21:45
Copy link
Contributor

@TatianaKapos TatianaKapos left a comment

Choose a reason for hiding this comment

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

LGTM!

@acoates-ms acoates-ms merged commit 19b6aba into microsoft:main Oct 24, 2024
59 checks passed
@acoates-ms acoates-ms deleted the shadowbox branch October 24, 2024 18:28
if (!viewProps.boxShadow.empty()) {
shadow.Offset({viewProps.boxShadow[0].offsetX, viewProps.boxShadow[0].offsetY, 0});
shadow.Opacity(1);
shadow.BlurRadius(viewProps.boxShadow[0].blurRadius);
Copy link
Collaborator

@NickGerleman NickGerleman Oct 25, 2024

Choose a reason for hiding this comment

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

Heads up that “blur radius” can mean different things depending on implementation.

Gaussian blur follows Gaussian distribution, where standard deviation can be represented as sigma. In blur function, that is number of pixels. The shadow is still visible outside this number of pixels, so different APIs might represent a radius differently.

More concretely:

  1. Apple shadow APIs all treat radius as sigma
  2. Android and legacy Skia blur related APIs map 1px radius to 0.57px sigma
  3. CSS drop shadow and blur filter treats radius as sigma
  4. CSS box shadows map 1px radius to 0.5px sigma. https://www.w3.org/TR/css-backgrounds-3/#shadow-blur

The exact algorithm is not defined; however the resulting shadow must approximate (with each pixel being within 5% of its expected value) the image that would be generated by applying to the shadow a Gaussian blur with a standard deviation equal to half the blur radius.

What this means in practice… there’s a good chance you need to halve this for box shadows, if composition works in Sigma (or do some special math if they do their own thing to map to css expectations).

acoates-ms added a commit to acoates-ms/react-native-windows that referenced this pull request Nov 2, 2024
* Add very basic box-shadow support

* Change files
acoates-ms added a commit that referenced this pull request Nov 5, 2024
* [Fabric] Introducing autocapitalize prop in TextInput - Take 2 (#13343)

* New implementation of autocapitalize!

* Change files

* Fixed bug for sentences scenario

* Just keep characters mode for now

* Revert "Just keep characters mode for now"

This reverts commit 60ca1ce.

* Re-apply changes minus packages.json.lock

* The original js file was deleted, re-applying changes

* Fixed snapshot and lint errors

* Fix override mismatch, added comments

* Remove stale test check

* Minor changes

* Update obsolete snapshot

* Cleanup ReactNativeAppBuilder and ReactNativeWin32App (#13983)

This PR simplifies and scopes down the API for `ReactNativeAppBuilder` and `ReactNativeWin32App`.

- Bug fix (non-breaking change which fixes an issue)

`ReactNativeAppBuilder`'s API surface made it too easy to call incorrectly and not realize it.

Resolves #13946

There are a variety of changes to the API surface:
* `ReactInstanceSettingsBuilder` deleted: not only are there are simply too many APIs that would need to be exposed to be useful, the very act of creating and replacing the app's `ReactNativeHost`'s `ReactInstanceSettings` with a new one is what caused the bug in #13946 in the first place
* `ReactNativeAppBuilder` now only exposes APIs to specify the intial, non-ReactNative, WinAppSDK types, (i.e. `DispatcherQueueController`, `Compositor`, and `AppWindow`), objects the app developer may already have created for their existing app, and otherwise is only responsible for building a `ReactNativeWin32App` with those types properly pre-made
* `ReactNativeWin32App::Start()` is now more responsible for the stitching together all of the relevant types to make a working Win32 fabric app
* All WinRT APIs without an immediate use-case have been commented out until we are sure they are necessary and that it is safe to expose them
* The template has been updated to follow the pattern of:
    * Use `ReactNativeAppBuilder` to get a `ReactNativeWin32` app with the base WinAppSDK types ready
    * Get and modify the types as necessary from the created app object (like the `ReactInstanceSettings` and the `AppWindow`)
    * Call `app.Start()`

N/A

Verified new apps and example apps in libraryes build and run properly.

Should this change be included in the release notes: _yes_

Cleanup ReactNativeAppBuilder and ReactNativeWin32App

* Update Guardrails on Provider Instantiation (#13961)

* Update Provider Guardrails

* Change files

* Format

* Update Snapshots

* Add very basic box-shadow support (#14028)

* Add very basic box-shadow support

* Change files

* Focus should notify island host when tab loop wraps to give host oportunity to take focus (#14026)

* Focus should notify island host when tab loop wraps to give host oportunity to take focus

* Change files

* Default scroll to bring a component into view should have padding around the viewport (#14018)

* Default scroll to bring a component into view should have padding around the viewport

* Change files

* Update focus visuals to use cornerRadius and inner/outer strokes (#14008)

* Update focus visuals to use cornerRadius and inner/outer strokes.

* Change files

* Format

* lint fix

* Scale focus border for scaleFactor

* [Fabric] Get Modal to host RN components in new hwnd (#13500)

* save state

* add example

* build but blank page still :(

* clean up comments

* visuals show up in new hwnd!

* clean up code

* better naming and unfork Modal examples

* testing save state

* Make the RN island a Modal member var

* Failed attempt at skipping root view in CEH, leaving it for learning purposes

* you can click on UI!

* clean up code

* Change files

* save state

* remove hardcoded rootTag

* add width/height to example

* add test

* revert simple.tsx

* remove test

* update snapshot

* feedback part 1: make Modal a RootComponentView

* feedback part2: simplify MountChildren

* fix deleting modal

* feedback round2

* remove comment

* remove imports

* feedback part 3

* fix overrides

* add simple layout - still has issues with padding/flex

* feedback part4

* lint

* update overrides

* Change files

* feedback

---------

Co-authored-by: Daniel Ayala <[email protected]>

* Support accessibilityState 'checked' (#13962)

* Implement accessibilityState checked

* Change files

* Add Testing

* Format and Update Snapshots

* Adjust Guardrails

* Merge

* Format

* Format

* Lint

* Change files

* Fix Merge Error

* Fix issue with prop cloning with custom native props (#14061)

* Fix issue with prop cloning with custom native props

* format

* prettier

* Change files

---------

Co-authored-by: React-Native-Windows Bot <[email protected]>

* change files

* Build fixes

* fix

* fix

* snapshot

---------

Co-authored-by: Daniel Ayala <[email protected]>
Co-authored-by: Jon Thysell <[email protected]>
Co-authored-by: Chiara Mooney <[email protected]>
Co-authored-by: Tatiana Kapos <[email protected]>
Co-authored-by: React-Native-Windows Bot <[email protected]>
acoates-ms added a commit to acoates-ms/react-native-windows that referenced this pull request Nov 12, 2024
* Add very basic box-shadow support

* Change files
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