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-consent: Add Didomi CMP documentation + example #22518

Closed
wants to merge 114 commits into from

Conversation

MaximePlancke
Copy link
Contributor

Add Didomi documentation and example to AMP related to this comment:
#21693 (comment)
#21752

caroqliu and others added 4 commits May 28, 2019 11:17
* Fallback

* Add fallback elements to example

* Lint

* Reset fallbackDisplayed_ when clearing items

* Add unit tests for fallback and check types

* Lint

* Lint

* Update tests

* New approach to fallback

* Remove fallback class

* Refactor tests

* Update test to reject promise instead of throw erro

* Update types

* Undo auto HTML lint

* Carlos comments

* Update unit test
* applyFillContent on amp-ad-custom

* Add examples.
@torch2424
Copy link
Contributor

@MaximePlancke

Per your comment in #21752 Feel free to @ mention me when things are finished up on your side, so I can take a look, review, and get this merged 😄

caroqliu and others added 18 commits May 28, 2019 16:41
…roject#21466)

* Display partial user input in bold for each filtered item

* Support substring highlighting for rich content

* /*OK*/ innerHTML; calls occur in mutate context

* Set default value for optional substring param

* Expose styling class

* Lint

* Reflect change in tests

* Lint

* Sanitize for plain text case

* Remove rich case

* Replace regex with node building implementation

* Remove sanitizeHtml

* Add highlight-user-entry attribute

* Add tests for both partial highlighting and not

* Lint

* Undo automatic HTML linting

* Update .out file to undo auto lint

* Add highlight-user-entry attribute to md
- Implement a single version of truncate, where the optional overflow
element is the last child of the component.
- Use overflow for truncation. Doing various optimizations (like using
Range) requires more work and has a lot of cases to consider. The
potential benefit is in the range of 5x-10x performance.
- Implement both a Shadow DOM and non-Shadow DOM version of the
component, with the Shadow DOM behind a separate flag.
  * The tests run against both versions.
- Add `slot` to validator global attributes.
* move markPageAsLoaded_ to buildCallback

* move mediaLayoutPromise declaration until it is needed

* change to a deferred pattern

* replace whenLoaded with CommonSignals.LOAD_END

* actually call media fn

* fix test in amp story to wait for page signals to end

* mark page as loaded right after media is loaded

* remove markpageasloaded

* Revert "remove markpageasloaded"

This reverts commit 7cf6f24.
…#22469)

* Support multi entitlement pingback

* add pingbackReturnsAllEntitlements support
* Change form attributes in example

* Allow relative src URLs
* Bug fix: use timer to track page engagement

* Adds support for tracking custom events
…ons (ampproject#22349)

* Started the allow list

* Implemented the old functionality in new allow list format

* Fixed up lintintg

* Fixed a bug so far

* Finished the allowed list

* Added / Fixed all tests for the attribute list

* Fixed up prettier linting

* Removed the test, can be added back once source is reviewed

* Made Requested offline changes

* Made requested changes

* Added/fixed tests for the current changes

* Fixed precommit checks

* Made comment changes

* Made PR Comments
* Fix invisible sidebar in email/actions.

* Fix lint.
* cl/249302791 Remove non-AMP formats for carousel reference points.

* cl/249465797 Revision bump for ampproject#22417

* cl/249525181 Revision bump for ampproject#22182

* cl/249558901 Revision bump for ampproject#22447

* cl/250316079 Revision bump for ampproject#22470

* cl/250386704 Revision bump for ampproject#22488
* Export `isDisabled` helper

The `isDisabled` helper will be used to determine if an element should
skip the dirtiness check.

* Split test cases to only test for one behavior

* s/fieldset/elementAncestralFieldset
* Extract `isFieldDefault` helper

Extract an common helper that can be used for form field dirtiness
check.

* Split text cases to only test for one behavior

* explicitly set state without relying on beforeEach

* move isFieldDefault to src/form.js

* Add comments on why elements are inserted as HTML
mister-ben and others added 5 commits June 6, 2019 09:23
…oject#22645)

* Remove AMP-Access-Control-Allow-Source-Origin from test server

* fix rebase
* Install Timer service directly instead of adopting

* fixes

* fix tests

* lints

* revert timer lookup

* review fixes
)

* If the viewer responds with something without a `stackIndex` (e.g. the value `true`), fall back to what we think the state should be.
* bypass xhr interceptor for amp-worker invocation

* fix type typo

* choumx comments

* add test

* fix check-types

* choumx comments
@zhouyx
Copy link
Contributor

zhouyx commented Jun 6, 2019

Thank you for your integration. LGTM! 🎉

@zhouyx
Copy link
Contributor

zhouyx commented Jun 6, 2019

I restarted the travis build. See different error
image

@estherkim This looks similar to the error I saw from #22696. Do you think rebase will solve the issue. Thank you!

* Use css.js for autoplay styles

* Rename generated CSS file
@estherkim
Copy link
Collaborator

It looks like that job skipped an important installation step (pip install --user protobuf in the before_install stage) and I don't know why. Can you try restarting the build again?

@zhouyx
Copy link
Contributor

zhouyx commented Jun 6, 2019

Restarted the build. no luck. @MaximePlancke could you please try rebase? Thank you

@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #22518 into master will decrease coverage by 6.97%.
The diff coverage is 82.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #22518      +/-   ##
==========================================
- Coverage   85.61%   78.64%   -6.98%     
==========================================
  Files        1556      801     -755     
  Lines      145994    49721   -96273     
==========================================
- Hits       124987    39101   -85886     
+ Misses      21007    10620   -10387
Flag Coverage Δ
#integration_tests 31.85% <0%> (-9.21%) ⬇️
#unit_tests 78.03% <82.35%> (-7.72%) ⬇️
Impacted Files Coverage Δ
src/utils/key-codes.js 100% <ø> (ø) ⬆️
...ons/amp-subscriptions/0.1/subscription-platform.js 0% <ø> (ø) ⬆️
src/service/standard-actions-impl.js 94.59% <ø> (ø) ⬆️
extensions/amp-analytics/0.1/vendors/permutive.js 100% <ø> (ø) ⬆️
src/service/hidden-observer-impl.js 92.85% <ø> (ø) ⬆️
extensions/amp-bind/0.1/bind-impl.js 80% <ø> (ø) ⬆️
src/service/timer-impl.js 91.48% <ø> (ø) ⬆️
src/service/navigation.js 87.28% <ø> (ø) ⬆️
src/service/action-impl.js 97.55% <ø> (ø) ⬆️
src/service/url-impl.js 95% <ø> (ø) ⬆️
... and 793 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fad19d9...8601b03. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 6, 2019

Codecov Report

Merging #22518 into master will decrease coverage by 6.97%.
The diff coverage is 82.35%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #22518      +/-   ##
==========================================
- Coverage   85.61%   78.64%   -6.98%     
==========================================
  Files        1556      801     -755     
  Lines      145994    49721   -96273     
==========================================
- Hits       124987    39101   -85886     
+ Misses      21007    10620   -10387
Flag Coverage Δ
#integration_tests 31.85% <0%> (-9.21%) ⬇️
#unit_tests 78.03% <82.35%> (-7.72%) ⬇️
Impacted Files Coverage Δ
src/utils/key-codes.js 100% <ø> (ø) ⬆️
...ons/amp-subscriptions/0.1/subscription-platform.js 0% <ø> (ø) ⬆️
src/service/standard-actions-impl.js 94.59% <ø> (ø) ⬆️
extensions/amp-analytics/0.1/vendors/permutive.js 100% <ø> (ø) ⬆️
src/service/hidden-observer-impl.js 92.85% <ø> (ø) ⬆️
extensions/amp-bind/0.1/bind-impl.js 80% <ø> (ø) ⬆️
src/service/timer-impl.js 91.48% <ø> (ø) ⬆️
src/service/navigation.js 87.28% <ø> (ø) ⬆️
src/service/action-impl.js 97.55% <ø> (ø) ⬆️
src/service/url-impl.js 95% <ø> (ø) ⬆️
... and 793 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fad19d9...8601b03. Read the comment docs.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: no and removed cla: yes labels Jun 6, 2019
@MaximePlancke
Copy link
Contributor Author

@zhouyx Sorry I think I rebase the wrong way I have all the commits (I am not use to work with fork and must have miss one step). I'll most likely close this PR and reopen one with the updated code

@zhouyx
Copy link
Contributor

zhouyx commented Jun 6, 2019

Sounds good. Thanks for letting me know.

@torch2424
Copy link
Contributor

@MaximePlancke Thanks for letting us know! 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet