Skip to content
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

✨ [amp-autocomplete] Add support for prefetch attribute for prefetching remote data #31021

Merged
merged 6 commits into from Nov 10, 2020

Conversation

dmanek
Copy link
Contributor

@dmanek dmanek commented Nov 6, 2020

  1. Implemented support for prefetch attribute
  2. Minor refactoring
  3. Added unit test

Resolves #24687

@amp-owners-bot amp-owners-bot bot requested a review from zhouyx November 6, 2020 00:53
@dmanek
Copy link
Contributor Author

dmanek commented Nov 6, 2020

cc @caroqliu

@zhouyx zhouyx requested a review from caroqliu November 6, 2020 19:07
Copy link
Contributor

@caroqliu caroqliu left a comment

Choose a reason for hiding this comment

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

LGTM, however note that users cannot start using this attribute until it is allowed in the validation rules. Please allow prefetch and update the validator tests too. :)

@dmanek
Copy link
Contributor Author

dmanek commented Nov 10, 2020

Thanks for your comments @caroqliu. I've updated the validation rules and added tests, PTAL.

Copy link
Contributor

@caroqliu caroqliu left a comment

Choose a reason for hiding this comment

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

Thank you! Looks good. Please also add this attribute to the documentation in extensions/amp-autocomplete/amp-autocomplete.md

@dmanek
Copy link
Contributor Author

dmanek commented Nov 10, 2020

Thanks for approving. I've already updated the documentation in amp-autocomplete.md 🙂

@caroqliu
Copy link
Contributor

Thanks for approving. I've already updated the documentation in amp-autocomplete.md 🙂

🤦‍♀️ You're right! Ignore me :)

@zhouyx zhouyx merged commit 2c64835 into ampproject:master Nov 10, 2020
@dmanek dmanek deleted the amp-autocomplete-prefetch branch November 17, 2020 05:12
twifkak pushed a commit to twifkak/amphtml that referenced this pull request Nov 18, 2020
@twifkak twifkak mentioned this pull request Nov 18, 2020
twifkak added a commit that referenced this pull request Nov 18, 2020
* cl/341717450 Revision bump for #31021

* cl/342729148 Revision bump for #31099

* cl/342767641 Revision bump for #31056

* cl/342951159 Fix blurry-placeholder test cases

* cl/342977800 Restricts value of transformed attribute

Co-authored-by: Michael Rybak <michaelrybak@google.com>
Co-authored-by: honeybadgerdontcare <sedano@google.com>
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
…hing remote data (ampproject#31021)

* Add support for `prefetch` attribute

* Update validation rules and add test

* Update validator-amp-autocomplete.out

* Address code review comments
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
* cl/341717450 Revision bump for ampproject#31021

* cl/342729148 Revision bump for ampproject#31099

* cl/342767641 Revision bump for ampproject#31056

* cl/342951159 Fix blurry-placeholder test cases

* cl/342977800 Restricts value of transformed attribute

Co-authored-by: Michael Rybak <michaelrybak@google.com>
Co-authored-by: honeybadgerdontcare <sedano@google.com>
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.

amp-autocomplete: Support prefetching remote data via an attribute to improve responsiveness for users
5 participants