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.

Sign up for GitHub

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

Jump to bottom

Revert part of #19947 and #19680 which caused race condition for amp-analytics layout #19963

Merged
merged 7 commits into from
Dec 22, 2018
Merged

Revert part of #19947 and #19680 which caused race condition for amp-analytics layout #19963

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c5a06d4
put amp-analytics > 3 viewports away in integration test
lannka Dec 18, 2018
175fd9d
Merge branch 'master' into analytics-3-viewport-test
lannka Dec 19, 2018
c5a733d
save a line
lannka Dec 19, 2018
a49103d
increase timeout, try to fix test
lannka Dec 19, 2018
8e284c7
Merge branch 'master' into analytics-3-viewport-test
lannka Dec 20, 2018
5993415
merge
lannka Dec 21, 2018
6871c4b
Revert part of #19947 and #19680
lannka Dec 21, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 1 addition & 6 deletions bundles.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@ exports.extensionBundles = [
{name: 'amp-ad-network-gmossp-impl', version: '0.1', type: TYPES.AD},
{name: 'amp-ad-exit', version: '0.1', type: TYPES.AD},
{name: 'amp-addthis', version: '0.1', type: TYPES.MISC},
{
name: 'amp-analytics',
version: '0.1',
options: {hasCss: true},
type: TYPES.MISC,
},
{name: 'amp-analytics', version: '0.1', type: TYPES.MISC},
{name: 'amp-anim', version: '0.1', type: TYPES.MEDIA},
{name: 'amp-animation', version: '0.1', type: TYPES.MISC},
{
Expand Down
18 changes: 15 additions & 3 deletions css/amp.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -662,9 +662,21 @@ amp-instagram {
/**
* Analytics & amp-story-auto-ads tags should never be visible. keep them hidden.
*/
amp-analytics:not(.i-amphtml-element),
amp-story-auto-ads:not(.i-amphtml-element){
display: none !important;
amp-analytics, amp-story-auto-ads {
/* Fixed to make position independent of page other elements. */
position: fixed !important;
top: 0 !important;
width: 1px !important;
height: 1px !important;
overflow: hidden !important;
visibility: hidden;
}

html.i-amphtml-fie > amp-analytics {
/* Remove position fixed if it's in iframe.
* So runtime can measure layoutBox correctly
*/
position: initial !important;
}

/**
Expand Down
36 changes: 0 additions & 36 deletions extensions/amp-analytics/0.1/amp-analytics.css
View file
Open in desktop

This file was deleted.

3 changes: 1 addition & 2 deletions extensions/amp-analytics/0.1/amp-analytics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import {Activity} from './activity-impl';
import {AnalyticsConfig, mergeObjects} from './config';
import {AnalyticsEventType} from './events';
import {CSS} from '../../../build/amp-analytics-0.1.css';
import {CookieWriter} from './cookie-writer';
import {
ExpansionOptions,
Expand Down Expand Up @@ -709,5 +708,5 @@ AMP.extension(TAG, '0.1', AMP => {
installVariableService(AMP.win);
installLinkerReaderService(AMP.win);
// Register the element.
AMP.registerElement(TAG, AmpAnalytics, CSS);
AMP.registerElement(TAG, AmpAnalytics);
});
14 changes: 0 additions & 14 deletions extensions/amp-story-auto-ads/0.1/amp-story-auto-ads.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,20 +20,6 @@
transform: scale(1.0) translateX(-100%) translateY(200%) !important;
}

/**
* amp-story-auto-ads tags should never be visible. keep them hidden.
*/
amp-story-auto-ads {
/* Fixed to make position independent of page other elements. */
position: fixed !important;
top: 0 !important;
left: 0 !important;
width: 1px !important;
height: 1px !important;
overflow: hidden !important;
visibility: hidden;
}

.i-amphtml-story-ad-link {
background-color: #FFFFFF !important;
border-radius: 20px !important;
Expand Down
2 changes: 2 additions & 0 deletions test/integration/test-amp-analytics.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ describe.configure().run('amp-analytics', function() {
// initialize _cid cookie with a CLIENT_ID
document.cookie='_cid=amp-12345';
</script>
<!-- put amp-analytics > 3 viewports away from viewport -->
<div style="height: 400vh"></div>
<amp-analytics>
<script type="application/json">
{
Expand Down