Skip to content

Pause/Resume and Layout/Unlayout callbacks#1637

Merged
jridgewell merged 11 commits intoampproject:masterfrom
jridgewell:conserveResourcesCallback
Mar 18, 2016
Merged

Pause/Resume and Layout/Unlayout callbacks#1637
jridgewell merged 11 commits intoampproject:masterfrom
jridgewell:conserveResourcesCallback

Conversation

@jridgewell
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell commented Jan 28, 2016

First pass at separating pauseCallback from unlayoutCallback, which adds resumeCallback to the mix.

  • Setting up the state machine needs work
  • Need integration tests on to check that doc/viewer visibility changes are detected and issue pause/resume and unlayout.

Fixes #2560.

Future PRs:

  • Move layoutCallback calls into the state machine transitions
  • Get rid of the boolean API from unlayoutCallback

@jridgewell jridgewell force-pushed the conserveResourcesCallback branch 2 times, most recently from 793eeb5 to 388fd0e Compare February 4, 2016 20:12
@jridgewell jridgewell force-pushed the conserveResourcesCallback branch 2 times, most recently from 73be9a0 to e9a23b1 Compare February 5, 2016 21:50
@jridgewell jridgewell force-pushed the conserveResourcesCallback branch 2 times, most recently from 1c8b8b9 to 7399177 Compare February 9, 2016 22:52
@jridgewell jridgewell assigned dvoytenko and unassigned cramforce Feb 10, 2016
@jridgewell
Copy link
Copy Markdown
Contributor Author

jridgewell commented Feb 10, 2016

It passed Travis!!! 🎆🎊

dvoytenko
dvoytenko reviewed Feb 10, 2016
View reviewed changes
src/base-element.js Outdated
Copy link
Copy Markdown
Contributor

@dvoytenko dvoytenko Feb 10, 2016

Choose a reason for hiding this comment

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

Unlayout before layout?

Copy link
Copy Markdown
Contributor Author

@jridgewell jridgewell Feb 12, 2016

Choose a reason for hiding this comment

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

Fixed to only be called if LAYOUT_COMPLETE.

@dvoytenko
Copy link
Copy Markdown
Contributor

dvoytenko commented Feb 12, 2016

@jridgewell to confirm, as discussed offline, let's do the PR where we add Viewer.getVisibilityState() with an appropriate callback and we'll circle back to this PR after?

@jridgewell
Copy link
Copy Markdown
Contributor Author

jridgewell commented Feb 13, 2016

let's do the PR where we add Viewer.getVisibilityState() with an appropriate callback and we'll circle back to this PR after?

Yup.

@jridgewell jridgewell force-pushed the conserveResourcesCallback branch 2 times, most recently from 59e82d6 to f102c53 Compare February 24, 2016 20:18
@jridgewell jridgewell mentioned this pull request Mar 1, 2016
Closed
@jridgewell jridgewell force-pushed the conserveResourcesCallback branch from f102c53 to 9bcae59 Compare March 8, 2016 18:30
@jridgewell jridgewell force-pushed the conserveResourcesCallback branch from d04ed7c to 1c237c3 Compare March 18, 2016 17:01
@jridgewell
Copy link
Copy Markdown
Contributor Author

jridgewell commented Mar 18, 2016

Once Travis passes, I'll merge.

@jridgewell jridgewell force-pushed the conserveResourcesCallback branch 2 times, most recently from e3b89ca to c975e8b Compare March 18, 2016 17:30
@jridgewell jridgewell force-pushed the conserveResourcesCallback branch from c975e8b to 1dbda46 Compare March 18, 2016 17:51
jridgewell added a commit that referenced this pull request Mar 18, 2016
@jridgewell
Merge pull request #1637 from jridgewell/conserveResourcesCallback
3ebc410 
Pause/Resume and Layout/Unlayout callbacks
@jridgewell jridgewell merged commit 3ebc410 into ampproject:master Mar 18, 2016
@jridgewell jridgewell deleted the conserveResourcesCallback branch March 18, 2016 18:27
@cramforce cramforce mentioned this pull request Mar 18, 2016
Closed
@jridgewell jridgewell mentioned this pull request Feb 3, 2017
Closed
@ampprojectbot ampprojectbot mentioned this pull request Jun 29, 2020
Closed
@ampprojectbot ampprojectbot mentioned this pull request Jan 28, 2021
Open
@ampprojectbot ampprojectbot mentioned this pull request Mar 4, 2021
Open
@elijahsoria elijahsoria mentioned this pull request May 5, 2021
Merged
@ampprojectbot ampprojectbot mentioned this pull request Jul 26, 2021
Open
This was referenced Jan 19, 2022
Open
Open
This was referenced Mar 17, 2023
Open
Open
Open
This was referenced Sep 5, 2023
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@dvoytenko dvoytenko

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jridgewell @dvoytenko @mkhatib @cramforce