Use @solana/react SelectedWalletAccountContextProvider in example rea…#1327
Conversation
|
0516fbf to
8828783
Compare
BundleMonUnchanged files (139)
No change in files bundle size Final result: ✅ View report in BundleMon website ➡️ |
mcintyre94
left a comment
There was a problem hiding this comment.
Thanks for this! I rebased on #1364 to fix an existing UI bug and this looks good to go
@illegalcall FYI since you opened this PR we updated our contributing guidelines and will no longer review PRs that are purely AI generated, as I think this probably was. We hope you'll keep contributing, but please review the guidelines before doing so.
|
Hey, thank you @mcintyre94. Secondly, Its great that you have guidelines about AI generated code, and I usually do use AI IDEs, to understand the codebase context and get familiar with a new codebase fast. But I believe that this was not purely AI generated. This was mostly generated by me and reviewed by AI. Also, the purely AI generated part of this would be the commit message. I think that was purely generated by AI. And with this message, I am not trying to push back. I am just sharing my perspective. |
|
🔎💬 Inkeep AI search and chat service is syncing content for source 'Solana Kit Docs' |
|
@illegalcall Totally fair, thanks for responding! The main reason in this specific case was because of the UI bug, we don't have unit tests so you wouldn't have been able to test this without that being fixed first. In future, based on our new guidelines, we would probably close a PR that could not/has not been tested/verified by a person, just because this makes reviewing them more difficult. The other thing is the PR description, I've mentioned in our guidelines a good rule of thumb is not to use AI to write that. I do appreciate that people have different levels of English, and we as reviewers can only review English messages, so this is a bit restrictive. But the reason is that AI PR descriptions tend to be more difficult to understand and review, while a person with the right context tends to write a message that is more helpful for us as reviewers. If you do have any suggestions about our guidelines, please feel free to open an issue and I'm definitely happy to discuss further. It might take some iteration to reach the right place here. I'm just keen to avoid getting into a back and forth here, since this is a merged PR and conversation here won't be very visible to anyone else :) Thanks again for contributing, and for reaching out! |
|
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
Problem
After #1154 introduced a reusable
SelectedWalletAccountContextProviderin@solana/react, the example React app should be updated to use it instead of maintaining app-local selected wallet account context/provider logic.Summary of Changes
SelectedWalletAccountContextProviderfrom@solana/reactinexamples/react-app/src/main.tsx.useSelectedWalletAccountin:examples/react-app/src/components/ConnectWalletMenu.tsxexamples/react-app/src/components/ConnectWalletMenuItem.tsxexamples/react-app/src/components/SignInMenu.tsxexamples/react-app/src/routes/root.tsxexamples/react-app/src/context/SelectedWalletAccountContext.tsxexamples/react-app/src/context/SelectedWalletAccountContextProvider.tsxValidation run:
pnpm --filter @solana/example-react-app test:typecheckpnpm --filter @solana/example-react-app compile:jsFixes #1183