Display Click to Load placeholders on demand #287
Merged
Conversation
This file contains hidden or 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
kzar
force-pushed
the
ctl-placeholder-on-block
branch
from
b1d9e28 to
0f17397
Compare
12 tasks
kzar
commented
Feb 27, 2023
kzar
commented
Feb 27, 2023
kzar
commented
Feb 27, 2023
kzar
commented
Feb 27, 2023
kzar
force-pushed
the
ctl-placeholder-on-block
branch
3 times, most recently
from
77d57ad to
cb97543
Compare
kzar
commented
Feb 28, 2023
kzar
commented
Feb 28, 2023
kzar
commented
Feb 28, 2023
kzar
commented
Feb 28, 2023
kzar
commented
Feb 28, 2023
kzar
force-pushed
the
ctl-placeholder-on-block
branch
from
cb97543 to
18796b1
Compare
kzar
commented
Mar 1, 2023
kzar
requested review from
jonathanKingston,
sammacbeth and
shakyShane
as code owners
ladamski
reviewed
Mar 1, 2023
Historically, the Click to Load placeholders were only drawn once at the point page load completed. That had two problems: 1. Sometimes placeholders would take too long to display, if the page took a long time to load fully. 2. Sometimes placeholders would not be displayed at all, if the blocked content was only created after page load had finished. To fix that, let's display the placeholders when instructed by the platform using a "displayClickToLoadPlaceholders" update message. Let's also restructure the response messages, so that they can be clearly distinguished.
kzar
force-pushed
the
ctl-placeholder-on-block
branch
from
18796b1 to
a035a24
Compare
|
Ok I'm having one issue but its annoying to repro. Basically after the browser has been open for a while (couple hours) CTL is no longer being applied to any page (whether I reload, open a new tab, etc). Maybe the background script is getting killed and not restarting? |
Weird. OK I have filed a task in Asana to investigate that. |
kzar
added a commit
to kzar/content-scope-scripts
that referenced
this pull request
Mar 3, 2023
This reverts commit 46ca757. The change was causing an integration test failure, which we ran out of time to debug. We will investigate and reapply this shortly.
kzar
added a commit
that referenced
this pull request
Mar 3, 2023
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.
Historically, the Click to Load placeholders were only drawn once at
the point page load completed. That had two problems:
took a long time to load fully.
blocked content was only created after page load had finished.
To fix that, let's display the placeholders when instructed by the
platform using a "displayClickToLoadPlaceholders" update message.
Let's also restructure the response messages, so that they can be
clearly distinguished.