fix(amp-user-notification): execute GET instead of POST on data-show-if-href #1321
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
erwinmombay
force-pushed
the
notify-get
branch
from
c45dde8
to
13b2981
Compare
|
Nit: This isn't really a refactor. More like a breaking change :) |
|
@cramforce lol good point 🎯 |
erwinmombay
force-pushed
the
notify-get
branch
from
13b2981
to
0b43e2c
Compare
erwinmombay
changed the title
erwinmombay
force-pushed
the
notify-get
branch
3 times, most recently
from
7ced1e6
to
93b0b44
Compare
erwinmombay
changed the title
|
@dvoytenko ready for review |
|
@dvoytenko PTAL |
| @@ -205,7 +205,7 @@ | |||
| <amp-user-notification | |||
| layout=nodisplay | |||
| id="amp-user-notification1" | |||
| data-show-if-href="http://localhost:8000/api/show" | |||
| data-show-if-href="http://localhost:8000/api/show?timestamp=$TIMESTAMP" | |||
There was a problem hiding this comment.
Remove $ - works without and we are trying to not advertise them.
erwinmombay
force-pushed
the
notify-get
branch
2 times, most recently
from
3750735
to
3951765
Compare
| describe('buildGetHref_', () => { | ||
|
|
||
| it('should do url replacement', () => { | ||
| dftAttrs['data-show-if-href'] = 'https://www.ampproject.org/path/?ord=$RANDOM'; |
|
Cool. Couple more comments... |
erwinmombay
force-pushed
the
notify-get
branch
2 times, most recently
from
081cc31
to
0f347d6
Compare
erwinmombay
force-pushed
the
notify-get
branch
from
0f347d6
to
76894dc
Compare
|
LGTM |
erwinmombay
added a commit
that referenced
this pull request
Jan 11, 2016
fix(amp-user-notification): execute GET instead of POST on data-show-if-href
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
closes #1228