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

Export dialog: add iOS to UI flow #30115

Merged
merged 6 commits into from Aug 9, 2019
Merged

Conversation

cpirich
Copy link
Contributor

@cpirich cpirich commented Aug 5, 2019

  • Expand Export dialog to handle the iOS UI flow (under a new, separate exportIOS experiment) - based on export spec: https://docs.google.com/document/d/1o-C8GOi45FCzUKPRnsyrNS8k10jUYOlrP2zIBPeO1X4/edit
  • NOTE: for the iOS case, there are a few notable differences
    1. An Apple developer account is required
    2. An Expo.io account is required
    3. Code Studio will create a snack (project) on Expo.io under the Expo "code.org" account, but will not generate the IPA. The 10-15 minute wait will not occur within Code Studio. The student will transition to the Expo.io site, which will remix the snack under the student's Expo.io account, explain the export process, prompt them to enter their Apple credentials, and show progress UI while waiting until the IPA is ready.
  • NOTE: the Expo snack-web changes are not yet available. They will be based on the approach in this branch (for anyone who is curious): expo/snack-web@master...cpirich:cpirich/routes-for-save-and-publishipa

New screenshots (for review by @ryansloan):
Screen Shot 2019-08-05 at 10 55 47 AM
Screen Shot 2019-08-05 at 10 56 04 AM
Screen Shot 2019-08-05 at 10 56 11 AM
Screen Shot 2019-08-05 at 10 56 23 AM

return;
}
// TODO: use new URL format once snack-web has been updated for this flow
// TODO: pass iconUri and splashImageUri to expo.io
Copy link
Contributor

Choose a reason for hiding this comment

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

Are there work items for these?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, for the link (below) should it redirect to the signup page? Right now, it sends me to a page with a bunch of code on it and I'm not sure what I'm supposed to do next.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, the snack-web changes are not yet available (that is what the TODO notes are indicating).
Once those changes are available on snack-web, this code will be updated to navigate to something like /publishipa/@:username/:projectName, which will redirect to signup/login and then explain the IPA export process.

</p>
<p style={commonStyles.text}>
<b>Note: </b>You must have an Apple developer account to create an
IPA using our Code.org partner site, Expo.io.
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems like this warning should be stronger @ryansloan - perhaps "REQUIRED: You must..."

@jmkulwik
Copy link
Contributor

jmkulwik commented Aug 5, 2019

Is there a design spec for the iOS route yet? I don't quite understand what the final product should look like? How much of this is going to be done by Snack Expo?

@cpirich
Copy link
Contributor Author

cpirich commented Aug 5, 2019

Is there a design spec for the iOS route yet? I don't quite understand what the final product should look like? How much of this is going to be done by Snack Expo?

@ryansloan can comment on the status of the spec. At this point, it looks like I will be doing the UI work on the Expo side based on the branch I referenced above. The Expo team is still working this week on completing the IPA build process.

@codecov-io
Copy link

codecov-io commented Aug 5, 2019

Codecov Report

❗ No coverage uploaded for pull request base (staging@96394aa). Click here to learn what that means.
The diff coverage is 55.93%.

Impacted file tree graph

@@            Coverage Diff             @@
##             staging   #30115   +/-   ##
==========================================
  Coverage           ?   68.21%           
==========================================
  Files              ?     1369           
  Lines              ?    84489           
  Branches           ?     3415           
==========================================
  Hits               ?    57637           
  Misses             ?    23755           
  Partials           ?     3097
Flag Coverage Δ
#integration 46.03% <0%> (?)
#storybook 56.6% <ø> (?)
#unit 58.02% <55.93%> (?)
Impacted Files Coverage Δ
apps/src/util/exporter.js 39.69% <0%> (ø)
...dio/components/ExportDialog/PublishAndroidPage.jsx 61.11% <100%> (ø)
...-studio/components/ExportDialog/GeneratingPage.jsx 39.28% <16.66%> (ø)
...de-studio/components/ExportDialog/PlatformPage.jsx 51.28% <25%> (ø)
...-studio/components/ExportDialog/PublishIOSPage.jsx 61.11% <53.84%> (ø)
...src/code-studio/components/ExportDialog/Dialog.jsx 62.43% <78.57%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 96394aa...99d6aa2. Read the comment docs.

@ryansloan
Copy link
Contributor

@jmkulwik @cpirich There is not a spec on the Expo end of this yet. In order to do testing of the iOS export process we need to hook up our end of this. The expo team has prototyped an "export for app store" button on the snack page, but we need to work on the actual UI plan. This is behind an experiment flag separate from our Android release and we're not planning to release iOS at the same time.

@cpirich cpirich merged commit 8c08777 into staging Aug 9, 2019
@cpirich cpirich deleted the cpirich/export-dialog-add-ios branch August 9, 2019 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants