This repository has been archived by the owner. It is now read-only.
refactor(client): Configurable marketing-snippet and marketing-mixin. #4519
Conversation
shane-tomlinson
force-pushed the
refactor-configurable-marketing-snippet
branch
2 times, most recently
from
62259d7
to
c947233
| }, | ||
|
|
||
| /** | ||
| * If if the browser is Firefox desktop |
vladikoff
Dec 20, 2016
Contributor
nit if if
nit if if
|
|
||
| createView(); | ||
| function testMarketingCampaign (marketingId) { | ||
| function testLinkEl(view, expectedType) { |
vladikoff
Dec 20, 2016
Contributor
nit: testLinkEl feels like a bad name. Can this be: assertLinkHasExpectedType or something?
nit: testLinkEl feels like a bad name. Can this be: assertLinkHasExpectedType or something?
|
forceUA works @shane-tomlinson left 2 nit comments, please merge once addressed |
This is an extraction from #4370 to make both simpler to review. For that PR, we need to be able to configure which app store logos are displayed to the user. This allows us to do so. The marketing-mixin module, instead of returning a mixin, returns a function that must be called with configuration options, the function returns the actual mixin. The marketing_snippet module allows two additional configuration options: * `marketingId` is the id used to log impressions and clicks * `which` forces specific logo(s) to be displayed. If not specified, uses default behavior. The Adjust links are extracted into Constants.js and shared between marketing_snippet.js and settings/clients.js. base.js->getSearchParam has been added to fetch a single search parameter. Add `isFirefoxAndroid`, `isFirefoxIos` and `isFirefoxDesktop` to user-agent.
shane-tomlinson
force-pushed the
refactor-configurable-marketing-snippet
branch
from
c947233
to
6e7e4bb
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
This is an extraction from #4370 to make both simpler to review. For
that PR, we need to be able to configure which app store logos are
displayed to the user. This allows us to do so.
The marketing-mixin module, instead of returning a mixin, returns
a function that must be called with configuration options, the function
returns the actual mixin.
The marketing_snippet module allows two additional configuration
options:
marketingIdis the id used to log impressions and clickswhichforces specific logo(s) to be displayed. If not specified,uses default behavior.
The Adjust links are extracted into Constants.js and shared between
marketing_snippet.js and settings/clients.js.
base.js->getSearchParam has been added to fetch
a single search parameter.
Add
isFirefoxAndroid,isFirefoxIosandisFirefoxDesktoptouser-agent.
@vladikoff - r?