Skip to content
This repository has been archived by the owner on Dec 19, 2024. It is now read-only.

Define path and query param propagation semantics. #40

Merged
merged 1 commit into from
Mar 17, 2016

Conversation

cdata
Copy link
Contributor

@cdata cdata commented Mar 11, 2016

Fixes #38

return;
}

this.set('route.__' + changes.path, changes.value);
Copy link
Contributor

Choose a reason for hiding this comment

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

will changes.path and changes.value exist even when the changes are splices? I guess we can probably ignore that case though, because we don't support arrays inside of queryParams currently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't, but a derived element might. But, maybe we should table implementation until we have true inheritance. WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Filed #42 so that we can fix later.

@rictic
Copy link
Contributor

rictic commented Mar 14, 2016

This looks really nice. Just a few comments.

@cdata cdata force-pushed the new-propagation-semantics branch from b4cd935 to 34fcdb5 Compare March 16, 2016 20:48
@cdata
Copy link
Contributor Author

cdata commented Mar 16, 2016

Filed PolymerElements/test-fixture#31 to address the dom-template inside test-fixture issue some day. Also left a note in the code referencing that issue.

@rictic
Copy link
Contributor

rictic commented Mar 17, 2016

LGTM!

cdata added a commit that referenced this pull request Mar 17, 2016
Define path and query param propagation semantics.
@cdata cdata merged commit 037fb91 into master Mar 17, 2016
@cdata cdata deleted the new-propagation-semantics branch March 17, 2016 22:48
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.

3 participants