Skip to content

Add boxShadow support for modern React Native styling#30

Open
fireinnti wants to merge 3 commits into
zeplin:mainfrom
fireinnti:main
Open

Add boxShadow support for modern React Native styling#30
fireinnti wants to merge 3 commits into
zeplin:mainfrom
fireinnti:main

Conversation

@fireinnti
Copy link
Copy Markdown

Change description

Adds box shadow property to react native extension.

Type of change

  • New feature (adds functionality)

Related issues

Fix #1

Checklists

Development

  • Lint rules pass locally
  • Application changes have been tested thoroughly
  • Automated tests covering modified code pass

Security

  • Security impact of change has been considered
  • Code follows company security practices and guidelines

Code review

  • Pull request has a descriptive title and context useful to a reviewer. Screenshots or screencasts are attached as necessary
  • "Ready for review" label attached and reviewers assigned
  • Changes have been reviewed by at least one other contributor
  • Pull request linked to task tracker where applicable

Co-authored-by: Copilot <copilot@github.com>
generateBoxShadowStyleObject(layer.shadows, layerType, context)
);

// Legacy shadow props need multiple layers. Box shadow can do all of them in one, but we want to keep the old ones for backwards compatibility
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should the legacy shadow properties come before, so that modern one takes precendence?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

My understanding is that this would have both showing. Do you think having one above the other would be better? I think that makes sense, but just making sure I'm tracking.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Showing both in the code output is OK. I first assumed shadows in React Native would work like how CSS properties work for standardized and vendor prefixed proprerties (i.e. the last written one takes precedence). However, if the system prioritizes the modern one over the legacy one no matter in which order they're written, this should be fine too.

I guess it wouldn't hurt to output the modern one below the legacy one though, your call. Just let me know, and we can merge it afterwards.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think I prefer the modern at the bottom, as the shadow box adds multiple properties, while the box shadow adds just one. So it's easier to glance at. Ultimately, I doubt it matters too much, so I'm down for the merge. Thanks for your input

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

If we both prefer modern at the bottom, would you mind updating the order of properties? Then we can merge quickly.

Copy link
Copy Markdown
Author

@fireinnti fireinnti May 7, 2026

Choose a reason for hiding this comment

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

Done, thanks!

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sorry for back and forth, but aren't the tests needed to be updated as well?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

You are absolutely correct. Updated the tests.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@merih, is everything good now?

Branden Ham and others added 2 commits May 7, 2026 11:15
…Shadow support

Co-authored-by: Copilot <copilot@github.com>
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.

Box-shadow not supported.

2 participants