Add new Messaging format to Click to Load #546
Conversation
…w messaging layer wrapper
franfaccin
force-pushed
the
ctl-add-new-msg-format
branch
from
2237327 to
77f1568
Compare
There was a problem hiding this comment.
I took a pass of the click-to-load.js changes @franfaccin. Some suggestions but mostly they look pretty good 👍 . I will leave the sendMessageWrapper.js to Shane.
There was a problem hiding this comment.
@franfaccin this is great! I've tried to reflect what we spoke about in our call - let me know if you think I missed anything.
Here's the branch I made in our call for reference: https://github.com/duckduckgo/content-scope-scripts/compare/shane/pr-review
…portConfig. Move messagingContext creation to ContentFeature. Rename lazy.messaging to ctl.messaging. Clean up request() code to handle getYouTubeVideoDetails requests better. Clean up the code.
…ickToLoadMessagingTransport to match sendMessage() messages format
Fix return type. Move `displayClickToLoadPlaceholders` subscribe up to avoid race conditions.
franfaccin
force-pushed
the
ctl-add-new-msg-format
branch
from
6519c27 to
8dc23c2
Compare
franfaccin
force-pushed
the
ctl-add-new-msg-format
branch
from
579b657 to
0caacb2
Compare
|
@franfaccin - I've approved this now - thanks for working through everyone's feedback ❤️! If you consider this complete, please merge when you are ready 💪🏻 Just a reminder too that we're attempting to keep |
Remove sendMessage calls from click-to-load.js and replace it with new messaging layer wrapper