Corrects logic error with visibilityPercentageMin/Max #12451
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jonkeller
changed the title
…er/amphtml into iframe-transport-documentation-2
jonkeller
commented
Dec 13, 2017
| @@ -375,7 +375,7 @@ The `visibilitySpec` is a set of conditions and properties that can be applied t | |||
| - `waitFor` This property indicates that the visibility trigger should wait for a certain signal before tracking visibility. The supported values are `none`, `ini-load` and `render-start`. If `waitFor` is undefined, it is defaulted to [`ini-load`](#initial-load-trigger) when selector is specified, or to `none` otherwise. | |||
| - `continuousTimeMin` and `continuousTimeMax` These properties indicate that a request should be fired when (any part of) an element has been within the viewport for a continuous amount of time that is between the minimum and maximum specified times. The times are expressed in milliseconds. The `continuousTimeMin` is defaulted to 0 when not specified. | |||
| - `totalTimeMin` and `totalTimeMax` These properties indicate that a request should be fired when (any part of) an element has been within the viewport for a total amount of time that is between the minimum and maximum specified times. The times are expressed in milliseconds. The `totalTimeMin` is defaulted to 0 when not specified. | |||
| - `visiblePercentageMin` and `visiblePercentageMax` These properties indicate that a request should be fired when the proportion of an element that is visible within the viewport is between the minimum and maximum specified percentages. Percentage values between 0 and 100 are valid. Note that the lower bound (`visiblePercentageMin`) is inclusive while the upper bound (`visiblePercentageMax`) is not. When these properties are defined along with other timing related properties, only the time when these properties are met are counted. They default to 0 and 100 when not specified. | |||
| - `visiblePercentageMin` and `visiblePercentageMax` These properties indicate that a request should be fired when the proportion of an element that is visible within the viewport is between the minimum and maximum specified percentages. Percentage values between 0 and 100 are valid. Note that the upper bound (`visiblePercentageMax`) is inclusive while the lower bound (`visiblePercentageMin`) is not. When these properties are defined along with other timing related properties, only the time when these properties are met are counted. They default to 0 and 100 when not specified. | |||
There was a problem hiding this comment.
Motivation:
#12426 (comment)
bradfrizzell
approved these changes
Dec 13, 2017
glevitzky
approved these changes
Dec 13, 2017
ghost
approved these changes
Dec 13, 2017
|
@jonkeller do you want to update the doc for the 0 and 100 edge cases? |
Will do. |
gzgogo
pushed a commit
to gzgogo/amphtml
that referenced
this pull request
Jan 26, 2018
* One possible fix to min viz 100 * Undo commit to master * Adds documentation of iframe transport * Adds note to why-not-iframe about iframe transport * Fix typo in link * Implements review feedback * Adds 2 sentences on prioritization * Corrects logic error in doc * Adds documentation of iframe transport * Adds note to why-not-iframe about iframe transport * Fix typo in link * Implements review feedback * Corrects logic error in doc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ignore the ton of commits - I had a little trouble initially with rebasing onto changes I made to this file a couple hours ago.