Docs Content Update #233

mikkoh · 2016-09-22T17:38:47Z

This pull request does the following:

Some updates to docs content
- Made sure there were more examples in the docs
- Examples actually have real world ids and data not just "1234" for ids
- The first example encountered in docs should be copy and pasteable
New docs created for some models
Re-order API docs in sidebar based on "importance" rather than sorting it alphabetically
Added a useable example to the Readme. (might not be bad to have 2 or three examples)

Would love if @minasmart @tessalt @yomexzo you could take a look. Unfortunately right now you cannot run npm run doc:build currently so to preview the docs in yuidoc just run:

node_modules/.bin/yuidoc --server 8080 -T default src/

I don't think this PR should be merged in until we can preview the actual docs but I figured I'd put this out there.

minasmart

This looks real good. I haven't done a full grammar check, but so far just that one things in your test.

minasmart · 2016-09-22T17:45:51Z

tests/unit/models/cart-model-test.js

+  const quantity = 2;
+
+  const variant = singleProductFixture.product_listing.variants[1];
+


This doesn't ensure an update was made. You should validate that line items is empty to start.

There will definitely be grammar mistakes 😬

Hmmm.. Ok. I'll take a look at the code for this test. This test was actually from master. I just literally just copied and pasted it to ensure the old naming of the function still worked and so did the new. (running the same tests with both functions)

@minasmart what I did instead was to get the initial quantity if a line item exists. Then add that to the quantity.

It just didn't feel right to test if it exists first.

Oh, I wasn't suggesting to test if it exists. I just meant that we should assert the transition, so test that our assumptions about initial state are valid, so that we can ensure that a change has occurred

So is it good now? If we take the initial quantity and ensure it has increased it should be good.

minasmart

Just a couple things. Otherwise, looks great!

minasmart · 2016-09-26T14:06:30Z

tests/unit/models/cart-model-test.js

@@ -73,20 +73,57 @@ test('it proxies sub total from the underlying cart', function (assert) {
  assert.equal(model.subtotal, cartFixture.cart.subtotal_price);
 });

-test('it creates a line item when you add a variant', function (assert) {
+test('it deprecates addVariants to createLineItemsFromVariants', function (assert) {


Honestly, instead of duplicating the test, I would stub createLineItemsFromVariants and test that it's called by the deprecated method. Right now, this doesn't test that. It just tests that they have similar functionality (assuming the test bodies are the same).

How would you go about testing that addVariants calls createLineItemsFromVariants?

My thought was for the end user it doesn't really matter as long as the functionality is the same.

How were you thinking of testing this @minasmart?

minasmart · 2016-09-26T14:07:59Z

README.md

+
+// fetch a product using resource id
+shopClient.fetchAllProducts()
+  .then(function (products) {


For the time being, I would match the style that's in the rest of the project:

doTheThing().then(() => { // ... }).then(() => { // ... });

Will do... I actually prefer that style vs whats in shopify/javascript

This should be fixed. https://github.com/Shopify/js-buy-sdk/pull/233/files#diff-04c6e90faac2675aa89e2176d2eec7d8R21

minasmart · 2016-09-26T14:08:15Z

docs/assets/scripts/docs.js

+      return items.find(function(docClass) {
+        return docClass.name === key;
+      });
+    })


Same here (match style)

Hey assuming in this case it's not adding a space after function anything else. Linting is totally broken on this file since it's es5.

This should be fixed. https://github.com/Shopify/js-buy-sdk/pull/233/files#diff-e5d789301f1d0e399ded55169fc3ccf0R13

mikkoh · 2016-09-26T18:15:33Z

@yomexzo any updates on getting master functional to build and serve docs?

yomexzo · 2016-09-29T18:14:17Z

@mikkoh, #205 should fix master. I just rebased and attended to comments on that PR.

…tly use the config and this is all documented in src/shopify.js

… Mina deprecated addVariants in favour of a new function called createLineItemsFromVariants which is more inline with naming conventions of other functions within this class

…odel which didn't exist before

…alphabetization

…ts were actually added

…mple in the guide

…anced for a starter guide

…ht set it's value manually

mikkoh · 2016-10-18T17:57:54Z

@minasmart @jamesmacaulay I'd say this PR is ready to go.

If you want to review the docs do:

npm run doc:build
npm run doc:serve

I think maybe the only thing that might be nice to add documentation to is:
createCart because it sais

attributes representing the internal state of the cart to be persisted to localStorage.

There should be an example explaining what attrs you'd pass to createCart

Or is this something we just remove from docs because it's "complex".

@minasmart could you add in the docs for this section if you feel it's important to keep?

mikkoh · 2016-10-18T18:42:28Z

@minasmart I added a note about sorting order to fetchQueryProducts. Is it possible to pass in sort_by to fetchQueryCollections or other functions?

I documented sort_by based on this issue:
#175

minasmart suggested changes Sep 22, 2016

View reviewed changes

minasmart suggested changes Sep 26, 2016

View reviewed changes

yomexzo force-pushed the docs-content branch from e818dc3 to a0d9c3e Compare September 26, 2016 17:12

yomexzo mentioned this pull request Oct 13, 2016

Rewrite API docs generation script into javascript from shell script #205

Merged

mikkoh mentioned this pull request Oct 14, 2016

Review & re-implement our docs for simplicity and encouraging contribution #202

Closed

8 tasks

mikkoh added 23 commits October 17, 2016 15:51

Merged in deprecated notices

0d09b19

Removed Config from public documentation as the user will never direc…

0b695d0

…tly use the config and this is all documented in src/shopify.js

Added documentation that is more useable to src/shop-client.js

e430773

Worked on documentation for cart-model.js. Also after discussion with…

808239f

… Mina deprecated addVariants in favour of a new function called createLineItemsFromVariants which is more inline with naming conventions of other functions within this class

Worked on documentation for cart-model.js

86fb51e

Removed logger from docs

06340f6

Made fetchAllProducts and fetchAllCollections be public in docs

6f5df3c

Added some documentation for product-model

bb6e762

Made attributes be properties for product-model

14fd070

Added an example and worked on documentation for product-variant-model

8fd619a

Added in formatted price

d00738c

Small edit to available doc

1352890

Added real world ids to examples

d1d346b

Adde back type into logger

8951e44

Worked on cart documentation. Added documentation to cart-line-item-m…

d078e4f

…odel which didn't exist before

Added copy and pasteable exampels to documentation

69d8b27

Worked on ordering documentation sidebar based on importance and not …

52057a4

…alphabetization

Fixed a small thing I dropped in

9ba88fc

Documented options

96293ed

Fixed some linting issues in COMMENTS 😬

5fc4836

Implemented getting initial quantity of item to check that the varian…

589d41c

…ts were actually added

Tried to match the style of the rest of the repo on these to scripts

2e54ee1

Added a note about installing via npm under 'Including the Buy SDK'

c7282b5

mikkoh added 4 commits October 17, 2016 15:56

Worked on documentation. Worked on cart and creating a buy button exa…

c9cddc9

…mple in the guide

Fixed select + options example

daf9ae7

Worked on documentation for options

f74df35

Deleted initializing a cart from existing data as it might be too adv…

072e1a6

…anced for a starter guide

mikkoh force-pushed the docs-content branch from 58406e9 to 072e1a6 Compare October 17, 2016 20:11

mikkoh added 3 commits October 17, 2016 16:27

Made logger and reference-model be private classes for documentation

d750f61

Updated yuidoc theme

eb4f962

just added a note that selections is read only. Otherwise someone mig…

f190542

…ht set it's value manually

Added a note about sorting options for fetchQueryProducts

3cefdf1

mikkoh force-pushed the docs-content branch from 258f6fa to 3cefdf1 Compare October 18, 2016 18:55

Made updateCart be private. Might be good to deprecate

1a80814

This was referenced Oct 26, 2016

Updated docs readme for local build #254

Merged

Unable to initialise cart with variants. Documentation example not working. #69

Closed

minasmart approved these changes Oct 26, 2016

View reviewed changes

mikkoh merged commit 8ccac57 into master Oct 27, 2016

mikkoh deleted the docs-content branch October 27, 2016 14:01

mikkoh temporarily deployed to production October 27, 2016 16:03 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docs Content Update #233

Docs Content Update #233

mikkoh commented Sep 22, 2016

minasmart left a comment

minasmart Sep 22, 2016

mikkoh Sep 22, 2016 •

edited

mikkoh Sep 23, 2016

minasmart Sep 26, 2016

mikkoh Sep 26, 2016

minasmart left a comment

minasmart Sep 26, 2016

mikkoh Sep 26, 2016

mikkoh Oct 18, 2016

minasmart Sep 26, 2016

mikkoh Sep 26, 2016

mikkoh Sep 26, 2016 •

edited

minasmart Sep 26, 2016

mikkoh Sep 26, 2016

mikkoh Sep 26, 2016

mikkoh commented Sep 26, 2016

yomexzo commented Sep 29, 2016

mikkoh commented Oct 18, 2016

mikkoh commented Oct 18, 2016

		const quantity = 2;

		const variant = singleProductFixture.product_listing.variants[1];

Docs Content Update #233

Docs Content Update #233

Conversation

mikkoh commented Sep 22, 2016

minasmart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikkoh Sep 22, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

minasmart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikkoh Sep 26, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mikkoh commented Sep 26, 2016

yomexzo commented Sep 29, 2016

mikkoh commented Oct 18, 2016

mikkoh commented Oct 18, 2016

mikkoh Sep 22, 2016 •

edited

mikkoh Sep 26, 2016 •

edited