Skip to content

codepen documentation update#375

Merged
indifferentghost merged 15 commits intomasterfrom
362-codepen
Aug 23, 2018
Merged

codepen documentation update#375
indifferentghost merged 15 commits intomasterfrom
362-codepen

Conversation

@indifferentghost
Copy link
Copy Markdown
Contributor

@indifferentghost indifferentghost commented Aug 22, 2018

Continuing to fix #362

This PR fixes the following issues and continues to update examples across the board.
There's an issue where @codepen doesn't work under @option.

can-define.types.propDefinition.html

  • default option shows the old value.
  • Code following "Within a property definition," isn't js.

can-define.types.default.html

  • codepen

can-define.types.defaultConstructor.html

  • codepen

can-define.types.get.html

  • codepen examples

can-define.types.set.html

  • Use should have codepen-able examples.

@indifferentghost
Copy link
Copy Markdown
Contributor Author

Continuing to fix.

can-define.types.type.html

Comment thread docs/types.get.md Outdated
compute( 2 );
map.value; //-> 2
```
<!--@codepen-->
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this the one that doesn't work?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in commit 753b914.

Comment thread docs/types.get.md

const Store = DefineMap.extend( {
locations: DefineList,
locations: { Default: DefineList },
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

locations: DefineList is short-hand for locations: { Type: DefineList }.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

If It's set like that this.locations is undefined in the locationIds getter function.
https://codepen.io/anon/pen/XPmmmZ

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The new keyword was needed in front of DefaultList. Fixed in 73cc278.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You don't need the new keyword with Default. The problem is just that this example doesn't work. I think your original change to locations: { Default: DefineList } is the correct way to do this. That is how you do what the description says

The following example creates an empty locationIds can-define/list/list when a new instance of Store is created.

Sorry to have led you down the wrong path.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Reverted these changes made in 73cc278 in ae37c1a.

Comment thread docs/types.propDefinition.md Outdated
value: 0
},
address: {
value: function() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this should be default: function() { ... }.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 1dd11b3.

@phillipskevin
Copy link
Copy Markdown
Contributor

Looks good @HTMLGhozt. Just a couple small things to fix up.

- Constructor needs `new` keyword in shorthand. I was unable to get it
to work properly by just assigning `DefaultList`.
- Now returning in map example to properly get and return keys.
Copy link
Copy Markdown
Contributor

@phillipskevin phillipskevin left a comment

Choose a reason for hiding this comment

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

Looks good. Nice work @HTMLGhozt.

@indifferentghost indifferentghost merged commit c9fe2e1 into master Aug 23, 2018
@indifferentghost indifferentghost deleted the 362-codepen branch August 24, 2018 17:15
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.

Documentation Issues

2 participants