-
Notifications
You must be signed in to change notification settings - Fork 7
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
codepen documentation update #375
Conversation
- The exception is identity and serialize - Examples indented by signatures and option don't seem to properly link
Continuing to fix. can-define.types.type.html
|
docs/types.get.md
Outdated
@@ -183,6 +190,7 @@ map.value; //-> 1 | |||
compute( 2 ); | |||
map.value; //-> 2 | |||
``` | |||
<!--@codepen--> |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in commit 753b914.
const Store = DefineMap.extend( { | ||
locations: DefineList, | ||
locations: { Default: DefineList }, |
There was a problem hiding this comment.
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 }
.
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
docs/types.propDefinition.md
Outdated
value: 0 | ||
}, | ||
address: { | ||
value: function() { |
There was a problem hiding this comment.
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() { ... }
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 1dd11b3.
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.
There was a problem hiding this 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.
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 oldvalue
.js
.can-define.types.default.html
can-define.types.defaultConstructor.html
can-define.types.get.html
can-define.types.set.html