Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Fix code samples for sub-property observer docs #1424

Merged
merged 7 commits into from
Nov 16, 2015
Merged

Fix code samples for sub-property observer docs #1424

merged 7 commits into from
Nov 16, 2015

Conversation

kaycebasques
Copy link
Contributor

Fixes #1384.

hallo @samccone can you code review the new code? Just thought I'd mix it up and interact with someone new :)

hallo @arthurevans I changed the content to be step-by-step instructions, let me know if this works.

@kaycebasques kaycebasques added this to the Q4S3 milestone Nov 11, 2015
Example:

Polymer({
1. Initialize your object to a default value.

Choose a reason for hiding this comment

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

These aren't really steps (or have a necessary order), so they'd be better as bullet points than numbered items.

#1 here isn't actually necessary. (For example, imagine if your user object is only initialized when the user is logged in. You can still set up an observer for that path—it just won't fire until you set the object for the first time.)

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 numbered list felt a little funky. Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✔️ changed list to unordered
✔️ removed stuff about initializing obj to default val

2. Define an `observers` array.
3. For each object sub-property that you want to observe, add an element to
the `observers` array. Each element must be a method call that
accepts a single argument. The argument is the path to the object

Choose a reason for hiding this comment

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

method call -> this is difficult one. What you're really specifying is a method name, followed by the path you want to observe in parenthesis. I wouldn't emphasize the "single argument" angle -- the observers array can be used to specify path observers, deep path observers, and multiple-property (or path) observers, like;

observers: [
  'observeSomeStuff(cats,dogs,primates.*,dolphins.bottlenose)'
]

In the existing docs, we've used the term "dependent property" (or path would be more accurate) to refer to the stuff inside an observer or computed property definition. The distinction is between the thing being observed (the dependent property/path) and the value sent to the observer method (the argument).

observers: [
'userNameChanged(user.name)'
],
/* Each method referenced in observers must be defined in
Copy link
Contributor

Choose a reason for hiding this comment

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

I think elsewhere in the docs we use // for comment style

Copy link
Contributor Author

Choose a reason for hiding this comment

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

✔️

@robdodson
Copy link
Contributor

LGTM.. well barring the possible comment style change

@arthurevans
Copy link

LGTM.

arthurevans pushed a commit that referenced this pull request Nov 16, 2015
Fix code samples for sub-property observer docs
@arthurevans arthurevans merged commit ea5fb47 into Polymer:master Nov 16, 2015
@kaycebasques kaycebasques deleted the kb-path-observers branch December 3, 2015 20:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants