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

Flutter api docs #2818

Merged
merged 18 commits into from
Jan 15, 2021
Merged

Flutter api docs #2818

merged 18 commits into from
Jan 15, 2021

Conversation

fjnoyp
Copy link
Contributor

@fjnoyp fjnoyp commented Dec 31, 2020

Docs for the Flutter API category

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@fjnoyp fjnoyp requested a review from a team December 31, 2020 21:29
Copy link
Member

@haverchuck haverchuck left a comment

Choose a reason for hiding this comment

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

See comments. Looks good overall.

@haverchuck haverchuck self-requested a review January 9, 2021 00:35
Copy link
Member

@haverchuck haverchuck left a comment

Choose a reason for hiding this comment

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

I'm approving this contingent on confirmation that all code fragments were tested.

@@ -0,0 +1,5 @@
<amplify-callout>

OIDC modes is not available just yet for Flutter. We are actively working on this.
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 multiple OIDC modes? We should either say OIDC modes are ... or OIDC mode is ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed

var operation = Amplify.API.query(
request: GraphQLRequest<String>(
document: graphQLDocument,
variables: {"id": "8e0dd2fc-2f4a-4dc4-b47f-2052eda10775"}));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to show how to retrieve this Id from the mutation result? Based on our example schema and selection set used in mutation, it should be consistent on how Id gets retrieved instead of using a dummy Id here.

Copy link
Contributor

Choose a reason for hiding this comment

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

This section actually starts with Now that you were able to make a mutation, take the 'id'... and references the previous section which demonstrated how to run a mutation and get the result.
iOS docs also pass in a dummy id like this for getTodo. Android passes an id variable but then is enclosed in a function that takes id as an argument. So would rather just keep this similar to iOS.

Copy link
Contributor

Choose a reason for hiding this comment

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

Since we only have advanced flows, and nowhere in our docs we are showing how to access the data, I think it's a huge value add here to show, and possibly in other examples as well (e.g. a query example)

Copy link
Contributor

Choose a reason for hiding this comment

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

Added details for how to work with the mutation response as discussed.

docs/lib/restapi/fragments/flutter/fetch.md Outdated Show resolved Hide resolved
docs/lib/restapi/fragments/flutter/getting-started.md Outdated Show resolved Hide resolved
docs/lib/restapi/fragments/flutter/getting-started.md Outdated Show resolved Hide resolved
docs/lib/restapi/fragments/ios/getting-started.md Outdated Show resolved Hide resolved
Copy link
Contributor

@jamesonwilliams jamesonwilliams left a comment

Choose a reason for hiding this comment

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

Overall, I think this needs one more round and then it should be ready to ship.

Many small suggestions inline. Please take as click-to-accept as many of the easy ones as you can.

I also think you can de-duplicate some existing text ("Upon completion..."). There are already a lot of copies of this same text in the docs content, and this PR is adding more.

docs/lib/graphqlapi/fragments/flutter/authz.md Outdated Show resolved Hide resolved
docs/lib/restapi/fragments/flutter/update.md Outdated Show resolved Hide resolved
docs/lib/graphqlapi/fragments/flutter/mutate-data.md Outdated Show resolved Hide resolved
docs/lib/graphqlapi/fragments/flutter/mutate-data.md Outdated Show resolved Hide resolved
docs/lib/graphqlapi/fragments/flutter/authz/30_multi.md Outdated Show resolved Hide resolved
Copy link
Contributor

@Ashish-Nanda Ashish-Nanda left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@Ashish-Nanda Ashish-Nanda merged commit 8263fd6 into aws-amplify:master Jan 15, 2021
dependencies:
flutter:
sdk: flutter
amplify_flutter: '<1.0.0'
Copy link
Contributor

Choose a reason for hiding this comment

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

should have been amplify_core

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.

5 participants