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

Delay unhiding body until Dynamic CSS is loaded #1452

Merged

Conversation

jridgewell
Copy link
Contributor

But only if we’ve included the extension.

Also included:

  • Divorces Element Services from Service Promises
  • Misc testing fixes
    • Fixes issue with useCompiledJS on the debug page
    • Adds opacity checks to #hidden and #visible matchers

function installDynamicClassesService(win) {
return getService(win, 'dynamic-css-classes', () => {
addRuntimeClasses(win);
return {};
Copy link
Member

Choose a reason for hiding this comment

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

❤️

Copy link
Member

Choose a reason for hiding this comment

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

Maybe add a short docstring.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@cramforce
Copy link
Member

Very nice!

@jridgewell
Copy link
Contributor Author

You didn't catch my Easter egg, though. Have an idea what to name the file?

@@ -0,0 +1,36 @@
/**
Copy link
Member

Choose a reason for hiding this comment

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

Name the file render-delaying-extensions.js.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@cramforce
Copy link
Member

LGTM, but please add the timeout to makeBodyVisible

@jridgewell jridgewell force-pushed the dynamic-css-classes-delay-body branch 3 times, most recently from 68049da to 68e6786 Compare January 19, 2016 23:07
This will allow me to add a Dynamic CSS promise without exporting a
custom element.
- Fixes issue with `useCompiledJS` on the debug page
- Adds `opacity` checks to `#hidden` and `#visible` matchers
@jridgewell jridgewell force-pushed the dynamic-css-classes-delay-body branch 2 times, most recently from 6b37e03 to 3dd3bc9 Compare January 20, 2016 00:15
But only if we’ve included the extension.
@jridgewell jridgewell force-pushed the dynamic-css-classes-delay-body branch from 3dd3bc9 to 3ce05ca Compare January 20, 2016 00:55
@jridgewell
Copy link
Contributor Author

PTAL

@cramforce
Copy link
Member

LGTM

jridgewell added a commit that referenced this pull request Jan 20, 2016
Delay unhiding body until Dynamic CSS is loaded
@jridgewell jridgewell merged commit 1683d8e into ampproject:master Jan 20, 2016
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.

2 participants