-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
UIExplorer component navigation support for an embedded simulator in the documentation site #3419
Conversation
…tion embedded simulator
@facebook-github-bot shipit |
By analyzing the blame information on this pull request, we identified @tadeuzagallo, @nicklockwood and @vjeux to be potential reviewers. |
Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/1474424536221110/int_phab to review. |
@@ -0,0 +1,10 @@ | |||
// | |||
// Extracted from https://github.com/dsibiski/react-native-userdefaults-ios |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mind adding the Facebook license header block here? cc @dsibiski
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why having this file at all rather than using RCTSettings
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
True, it's best to use RCTSettings
for this simple use case. My lib will end up being used for those who need to use custom suites.
@jsierles updated the pull request. |
I'm sorry but I'm on pto for two weeks, won't be able to review this :( @brentvatne can you help? |
@vjeux - ok on it |
Looks like there are still some outstanding changes as per @tadeuzagallo's comments, @dsibiski @jsierles - ping me on Discord if you disagree with them and we can discuss |
Yeah, we'll go over these soon, just haven't had time to review yet. |
Any updates on this? |
@christopherdro has been working on upgrading the website code to help finish this off here: #4570 We'll take a look this week to get this finished up and on all the documentation pages. |
@jsierles Looking forward to this. This'll be so helpful. |
@jsierles do you have any updates for this pull request? It's been a while so we wanted to check in and see if you've looked at the requested changes. |
@jsierles Is this superseded by Christine's PR to add the simulator to all docs pages? Please reopen if that's not the case and you plan to continue working on this PR. |
No description provided.