Skip to content

Conversation

@MananTank
Copy link
Member

@MananTank MananTank commented Jun 6, 2024

PR-Codex overview

This PR fixes the issue where the custom theme was not applied in the ConnectEmbed loading screen in ConnectEmbed.tsx.

Detailed summary

  • Fixed custom theme not being applied in ConnectEmbed loading screen
  • Added CustomThemeProvider import and usage
  • Wrapped EmbedContainer in CustomThemeProvider with the provided theme or default to "dark"

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@vercel
Copy link

vercel bot commented Jun 6, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
thirdweb_playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 6, 2024 8:48pm

@changeset-bot
Copy link

changeset-bot bot commented Jun 6, 2024

🦋 Changeset detected

Latest commit: 7dc8ed0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages
Name Type
thirdweb Patch
@thirdweb-dev/sdk Patch
@thirdweb-dev/cli Patch
@thirdweb-dev/react-core Patch
@thirdweb-dev/react Patch
@thirdweb-dev/unity-js-bridge Patch
@thirdweb-dev/wallets Patch
@thirdweb-dev/auth Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed-hq
Copy link

codspeed-hq bot commented Jun 6, 2024

CodSpeed Performance Report

Merging #3240 will degrade performances by 14.12%

Comparing mnn/fix-initial-embed-theme (7dc8ed0) with main (dbf74aa)

Summary

❌ 1 regressions
✅ 8 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main mnn/fix-initial-embed-theme Change
keccakId 1.3 ms 1.5 ms -14.12%

@github-actions
Copy link
Contributor

github-actions bot commented Jun 6, 2024

size-limit report 📦

Path Size Loading time (3g) Running time (snapdragon) Total time
thirdweb (esm) 40.37 KB (0%) 808 ms (0%) 694 ms (-33.33% 🔽) 1.6 s
thirdweb (cjs) 89.34 KB (0%) 1.8 s (0%) 1.4 s (+3.87% 🔺) 3.2 s
thirdweb (minimal + tree-shaking) 4.75 KB (0%) 95 ms (0%) 31 ms (-63.26% 🔽) 126 ms
thirdweb/chains (tree-shaking) 423 B (0%) 10 ms (0%) 9 ms (-36.73% 🔽) 19 ms
thirdweb/react (minimal + tree-shaking) 15.75 KB (0%) 315 ms (0%) 126 ms (+5.63% 🔺) 441 ms

@MananTank
Copy link
Member Author

/release-pr

<EmbedContainer modalSize={modalSize}>
<LoadingScreen />
</EmbedContainer>
<CustomThemeProvider theme={props.theme || "dark"}>
Copy link
Member

Choose a reason for hiding this comment

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

feels like we should be passing the theme object around. doing that for RN

@joaquim-verges joaquim-verges merged commit d488223 into main Jun 8, 2024
@joaquim-verges joaquim-verges deleted the mnn/fix-initial-embed-theme branch June 8, 2024 03:13
@jnsdls jnsdls mentioned this pull request Jun 8, 2024
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