Skip to content

Reduce the number of recalc styles during a typically AMP boot procedure from 4 to 2#626

Merged
cramforce merged 1 commit intoampproject:masterfrom
cramforce:extra-recalc
Oct 15, 2015
Merged

Reduce the number of recalc styles during a typically AMP boot procedure from 4 to 2#626
cramforce merged 1 commit intoampproject:masterfrom
cramforce:extra-recalc

Conversation

@cramforce
Copy link
Copy Markdown
Member

which is likely the minimum possible right now.

Refactors both viewport and resources to use the service pattern
instead of being magic global singletons.

src/viewport.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

(this.scrollTop_ === null)?

Otherwise will keep requesting for 0.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done.

@cramforce
Copy link
Copy Markdown
Member Author

PTAL

src/viewport.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Here and below, {} style guide for if.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

i'll add it to the linter

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nvm, its already there [02:10:02] /home/travis/build/ampproject/amphtml/src/viewport.js: line 141, col 5, Error - Expected { after 'if' condition. (curly)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Wow, I'm not usually this sloppy :)

@dvoytenko
Copy link
Copy Markdown
Contributor

LGTM. Not sure what's in Travis.

@erwinmombay
Copy link
Copy Markdown
Member

@cramforce can you add a skip to that carousel test, will investigate and fix later.

procedure from 4 to 2 which is likely the minimum possible right
now.

Refactors both viewport and resources to use the service pattern
instead of being magic global singletons.
cramforce added a commit that referenced this pull request Oct 15, 2015
Reduce the number of recalc styles during a typically AMP boot procedure from 4 to 2
@cramforce cramforce merged commit 35f13b5 into ampproject:master Oct 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants