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

[RN] Add an option for staging the React via AppHub #1817

Merged
merged 9 commits into from
Sep 14, 2016
Merged

Conversation

orta
Copy link
Contributor

@orta orta commented Aug 30, 2016

This adds an admin panel that defaults to off

screen shot 2016-09-09 at 5 03 59 pm

Toggling it adds:

screen shot 2016-09-09 at 5 04 58 pm

Which offers a way to see:

  • When the current build of RN was deployed, as it's continuously deployed, this is pretty often
  • What version of Emission is running
  • An option to open a React component
  • The ability to run any specific build
  • A way to turn it off

This links to :head, which can mean that the Eigen Obj-C Emission can be out of date with what Emission expects to be exposed to the JS. E.g. the module providers here:

head
screen shot 2016-09-09 at 5 09 49 pm

eigen
screen shot 2016-09-09 at 5 10 08 pm

Do I have any long term answers, not really, I mean we did do a minor patch for this, so it does make sense. I've noted in slack that maybe an answer is to have deploys of Emission that are only the obj-c and to keep the same JS shipped with it, and have that run on eigen.

It's hard to say whether we'd expect a lot of change over the bridging there.


Opening any container

screen shot 2016-09-09 at 5 14 16 pm

I've got Eigen grabbing the list of exposed containers in the app registry, and puts those as clickable buttons. Then you can input any initial JSON state you want,

screen shot 2016-09-09 at 5 15 17 pm

which will push you through into that view controller

screen shot 2016-09-09 at 5 15 25 pm

this should make it much easier for beta testers to try out views before they're shipped into our ARSwitchboard.


TODO:

  • Verifying it works
  • The AppHub dependency needs to check for what subspecs it needs, currently it drags in the rest of React Native, which isn't optimal. PR sent Use React/Core for the podspec dependency AppHubPlatform/apphub#32
  • undoing the damage for space commander on ARAppDelegate+Emission.m
  • Ship a version of Emission that updates the Obj-C bridging side

Other than that, this is all you need to run the master commit for artsy/Emission inside eigen

@mention-bot
Copy link

@orta, thanks for your PR! By analyzing the annotation information on this pull request, we identified @ashfurrow to be a potential reviewer

@ArtsyOpenSource
Copy link
Contributor

ArtsyOpenSource commented Aug 30, 2016

✅ Good on 'ya.
No CHANGELOG changes made
2 Warnings
⚠️ Needs testing on a Phone if change is non-trivial
⚠️ Detected some Swift building time outliers
PR is classed as Work in Progress
Time Class Function
211.4ms SwiftyJSON.swift get {}
177.3ms WebSocket.swift private func processRawMessage(buffer: UnsafePointer, bufferLen: Int)
88.1ms Observable.swift private final func nextTokenHash() -> Int
77.3ms AuctionLotMetadataStackScrollView.swift required init(viewModel: LiveAuctionLotViewModelType, salesPerson: LiveAuctionsSalesPersonType, sideMargin: String)
73.0ms LiveAuctionsAdminViewController.swift @objc override func viewDidLoad()
67.6ms UIViewController+BlurredStatusView.swift func ar_presentBlurredOverlayWithTitle(title: String, subtitle: String, buttonState: BlurredStatusOverlayViewCloseButtonState = default)
65.5ms UIViewController+BlurredStatusView.swift (closure)
64.9ms LiveAuctionLotViewController.swift @objc override func viewDidLoad()
58.7ms RefinementOptionsViewController+Private.swift func stackView() -> ORStackView

Generated by 🚫 danger

@orta orta changed the title [WIP][RN] Add an option for staging the React via AppHub [RN] Add an option for staging the React via AppHub Sep 9, 2016
@alloy
Copy link
Contributor

alloy commented Sep 14, 2016

Impressive 👏

Let me know when this good to go!

@orta
Copy link
Contributor Author

orta commented Sep 14, 2016

OK, this is good to go

@alloy
Copy link
Contributor

alloy commented Sep 14, 2016

Sahweet!

@alloy alloy merged commit 76200df into master Sep 14, 2016
@alloy alloy deleted the orta-apphub branch September 14, 2016 14:40
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.

4 participants