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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 amp-story-desktop-one-panel background-blur Update blur when UI type is updated #35279

Merged
merged 2 commits into from
Jul 19, 2021

Conversation

processprocess
Copy link
Contributor

Context / Fixes #35269

Updating blur is currently called only during page change.

This PR will also update the blur when UI type is changed.
This ensures that the blur updates when resizing a window.

@amp-owners-bot
Copy link

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story.js

@@ -1846,6 +1846,9 @@ export class AmpStory extends AMP.BaseElement {
if (!this.backgroundBlur_) {
this.backgroundBlur_ = new BackgroundBlur(this.win, this.element);
this.backgroundBlur_.attach();
if (this.activePage_) {
this.backgroundBlur_.update(this.activePage_.element);
Copy link
Contributor

Choose a reason for hiding this comment

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

If a media from this page is already used for the blur, will it early return or animate again?

Copy link
Contributor Author

@processprocess processprocess Jul 19, 2021

Choose a reason for hiding this comment

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

I don't think there is currently any case that there is a double call since it's called within switchTo and onUIStateUpdate_.

If the window is scaled and still on the same page, It will draw the blur again but will not animate since it's the first update of the instance.
We remove the blur from the dom at the start of every onUIStateUpdate_:
https://github.com/ampproject/amphtml/blob/main/extensions/amp-story/1.0/amp-story.js#L1824-L1825

We currently don't check or cache page media within backgroundBlur but that might be a good thing to do if want to be sure to prevent duplicate calls.

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-story-desktop-one-panel background-blur activate blur when sizing up window
3 participants