Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: fix abtestui overlay #126

Merged
merged 18 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .nvmrc
Original file line number Diff line number Diff line change
@@ -1 +1 @@
v16.18.0
v20
2 changes: 1 addition & 1 deletion config/webpack.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module.exports = {
}, {
test: /\.css$/,
use: [{
loader: 'css-to-string-loader'
loader: 'to-string-loader'
}, {
loader: 'css-loader'
}]
Expand Down
26,571 changes: 14,061 additions & 12,510 deletions package-lock.json

Large diffs are not rendered by default.

25 changes: 13 additions & 12 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,25 +35,26 @@
"@semantic-release/changelog": "^6.0.1",
"@semantic-release/git": "^10.0.1",
"@types/jest": "^24.0.18",
"@types/node": "^12.7.8",
"@types/node": "^20.12.12",
fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
"ajv": "^6.10.2",
"copyfiles": "^2.1.1",
"css-loader": "^3.2.0",
"css-to-string-loader": "0.1.3",
"file-loader": "^4.2.0",
"html-webpack-plugin": "3.2.0",
"css-loader": "^7.1.1",
"file-loader": "^6.2.0",
"html-webpack-plugin": "^5.6.0",
"husky": "^3.0.5",
"jest": "^24.9.0",
"jest": "^29.1.2",
"jest-environment-jsdom": "^29.7.0",
"rimraf": "^3.0.0",
"semantic-release": "^19.0.5",
"terser-webpack-plugin": "^2.1.0",
"ts-jest": "^24.1.0",
"ts-loader": "^6.1.2",
"ts-jest": "^29.1.2",
"ts-loader": "^9.5.1",
"tslint": "^5.20.0",
"typescript": "^3.6.3",
"webpack": "^4.41.0",
"webpack-cli": "^3.3.9",
"webpack-dev-server": "^3.8.1"
"typescript": "^5.4.5",
"webpack": "^5.91.0",
"webpack-cli": "^5.1.4",
"webpack-dev-server": "^5.0.4",
"to-string-loader": "^1.2.0"
},
"files": [
"dist",
Expand Down
3 changes: 2 additions & 1 deletion src/config.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { Condition } from './condition';
import { removeAbTestParameter } from './query';
import { ConsoleTracking, Tracking } from './tracking';
import { CookiePersister, UserSessionPersister } from './userSessionPersister';
import { CookiePersister } from './userSessionPersister';
import type { UserSessionPersister } from './userSessionPersister';

export interface Config {
cookieName: string;
Expand Down
3 changes: 2 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,8 @@ import {
} from './main';
import { SplitTest } from './splitTest';
import { uiFactory } from './ui';
import { CookiePersister, UserSessionPersister } from './userSessionPersister';
fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
import { CookiePersister } from './userSessionPersister';
import type { UserSessionPersister } from './userSessionPersister';

const ui = uiFactory(
testsObservable,
Expand Down
20 changes: 19 additions & 1 deletion src/main.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import _getUserAgentInfo from './userAgentInfo';
import userSession, { UserSession } from './userSession';

const userAgentInfo = _getUserAgentInfo();
let configLoaded = false;
fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
export const tests: SplitTest[] = [];
export const testsObservable: BehavioralSubject<SplitTest[]> = new BehavioralSubject(tests);

Expand All @@ -33,8 +34,12 @@ export function config(userConfig: Partial<Config> = {}) {
if (userConfig.sessionPersister) {
const session = _config.sessionPersister.loadUserSession() || '';
_config.sessionPersister = userConfig.sessionPersister;
_config.sessionPersister.saveUserSession(session, _config.userSessionDaysToLive);
_config.sessionPersister.saveUserSession(
session,
_config.userSessionDaysToLive,
);
}
configLoaded = true;
}

/**
Expand Down Expand Up @@ -123,7 +128,20 @@ export function reset(): void {
_config.onVariationChange();
}

const waitUntil = (condition: () => any, checkInterval = 100) => {
return new Promise<void>((resolve) => {
const interval = setInterval(() => {
if (!condition()) {
return;
}
clearInterval(interval);
resolve();
}, checkInterval);
});
};

export async function shouldShowUI() {
await waitUntil(() => configLoaded);
fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
const promises = [
_config.globalCondition(userAgentInfo),
_config.uiCondition(userAgentInfo),
Expand Down
10 changes: 2 additions & 8 deletions src/styles/ui.css
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
transition: all .5s ease-out;
opacity: 1;
border-radius: 3px;
overflow: auto;
font-size: 12px;
}

.skift li.selected {
Expand All @@ -36,10 +38,6 @@
opacity: 0;
}

.skift .tests {
overflow-y: auto;
}

.skift .variations {
background: #f4f7f9;
box-shadow: inset 0 0 1px 1px #dfe2e5;
Expand Down Expand Up @@ -95,11 +93,7 @@
}

.skift .icon {
height: 16px;
width: 16px;
display: inline-block;
vertical-align: middle;
margin: 0 4px;
cursor: pointer;
color: #0c59f2;
border: none;
Expand Down
41 changes: 13 additions & 28 deletions src/ui.dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ export const uiFactory = (
}

function renderLink(splitTest: SplitTest, variation: InternalVariation) {
return renderButton(require('./images/link.svg'), () => {
const input = document.createElement('input');
input.value = splitTest.getVariationUrl(variation.name);
document.body.appendChild(input);
input.select();
return renderButton('currently active', () => {
fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
const button = document.createElement('button');
button.value = splitTest.getVariationUrl(variation.name);
document.body.appendChild(button);
document.execCommand('copy');
document.body.removeChild(input);
document.body.removeChild(button);
});
}

Expand All @@ -57,15 +56,10 @@ export const uiFactory = (
) {
const item = document.createElement('li');
item.textContent = variation.name;
const open = renderButton(require('./images/open.svg'), () => {
const open = renderButton('change to this variant', () => {
setCurrentTestVariation(splitTest.name, variation.name);
});

const link = renderLink(splitTest, variation);

item.appendChild(open);
item.appendChild(link);

return item;
}

Expand All @@ -89,8 +83,8 @@ export const uiFactory = (
.map(
(key) => `
<div>
<span class="data-label">${key}</span>
<span class="data-value">${data[key]}</span>
<span class='data-label'>${key}</span>
<span class='data-value'>${data[key]}</span>
</div>
`,
)
Expand Down Expand Up @@ -121,19 +115,6 @@ export const uiFactory = (
variations.appendChild(list);

return [test, variations];
} else {
const canRun = await splitTest.shouldRun(getUserAgentInfo());
const test = document.createElement('div');
test.className = 'test';
test.innerHTML = `
<div>Test <span class="data-value">${splitTest.name}</span> is not initialized</div>
<div>
<span class="data-label">Can run</span>
<span class="data-value">${canRun}</span>
</div>
`;

return [test];
}
}

Expand Down Expand Up @@ -166,12 +147,16 @@ export const uiFactory = (
.reduce((promise, futureElement) => {
return promise.then((elements) => {
return futureElement.then((element) => {
elements.push(...element);
if (element && elements) {
elements.push(...element);
}
return elements;
});
});
}, Promise.resolve([]));
if (test) {
test.forEach((x) => testList.appendChild(x));
}
});

const button = document.createElement('button');
Expand Down
22 changes: 17 additions & 5 deletions src/userAgentInfo.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
function getNameAndVersion() {
const ua = navigator.userAgent;
let tem: RegExpMatchArray | null;
let match: RegExpMatchArray = ua.match(/(opera|chrome|safari|firefox|msie|trident(?=\/))\/?\s*(\d+)/i) || [];
let tem: RegExpMatchArray | [] | null;
let match: RegExpMatchArray | [] =
ua.match(
/(opera|chrome|safari|firefox|msie|trident(?=\/))\/?\s*(\d+)/i,
) || [];
if (/trident/i.test(match[1])) {
tem = /\brv[ :]+(\d+)/g.exec(ua) || [];
return {
name: 'IE',
version: (tem[1] || ''),
version: tem[1] || '',
};
}
if (match[1] === 'Chrome') {
Expand All @@ -18,7 +21,9 @@ function getNameAndVersion() {
};
}
}
match = match[2] ? [match[1], match[2]] : [navigator.appName, navigator.appVersion, '-?'];
match = match[2]
? [match[1], match[2]]
: [navigator.appName, navigator.appVersion, '-?'];
tem = ua.match(/version\/(\d+)/i);
if (tem !== null) {
match.splice(1, 1, tem[1]);
Expand All @@ -33,7 +38,14 @@ function isMobile() {
const ua = navigator.userAgent || navigator.vendor;
// Disable the rule for now and consider using RegExp constructor with a string.
// tslint:disable-next-line:max-line-length
return /(android|bb\d+|meego).+mobile|avantgo|bada\/|blackberry|blazer|compal|elaine|fennec|hiptop|iemobile|ip(hone|od)|iris|kindle|lge |maemo|midp|mmp|mobile.+firefox|netfront|opera m(ob|in)i|palm( os)?|phone|p(ixi|re)\/|plucker|pocket|psp|series(4|6)0|symbian|treo|up\.(browser|link)|vodafone|wap|windows ce|xda|xiino/i.test(ua) || /1207|6310|6590|3gso|4thp|50[1-6]i|770s|802s|a wa|abac|ac(er|oo|s\-)|ai(ko|rn)|al(av|ca|co)|amoi|an(ex|ny|yw)|aptu|ar(ch|go)|as(te|us)|attw|au(di|\-m|r |s )|avan|be(ck|ll|nq)|bi(lb|rd)|bl(ac|az)|br(e|v)w|bumb|bw\-(n|u)|c55\/|capi|ccwa|cdm\-|cell|chtm|cldc|cmd\-|co(mp|nd)|craw|da(it|ll|ng)|dbte|dc\-s|devi|dica|dmob|do(c|p)o|ds(12|\-d)|el(49|ai)|em(l2|ul)|er(ic|k0)|esl8|ez([4-7]0|os|wa|ze)|fetc|fly(\-|_)|g1 u|g560|gene|gf\-5|g\-mo|go(\.w|od)|gr(ad|un)|haie|hcit|hd\-(m|p|t)|hei\-|hi(pt|ta)|hp( i|ip)|hs\-c|ht(c(\-| |_|a|g|p|s|t)|tp)|hu(aw|tc)|i\-(20|go|ma)|i230|iac( |\-|\/)|ibro|idea|ig01|ikom|im1k|inno|ipaq|iris|ja(t|v)a|jbro|jemu|jigs|kddi|keji|kgt( |\/)|klon|kpt |kwc\-|kyo(c|k)|le(no|xi)|lg( g|\/(k|l|u)|50|54|\-[a-w])|libw|lynx|m1\-w|m3ga|m50\/|ma(te|ui|xo)|mc(01|21|ca)|m\-cr|me(rc|ri)|mi(o8|oa|ts)|mmef|mo(01|02|bi|de|do|t(\-| |o|v)|zz)|mt(50|p1|v )|mwbp|mywa|n10[0-2]|n20[2-3]|n30(0|2)|n50(0|2|5)|n7(0(0|1)|10)|ne((c|m)\-|on|tf|wf|wg|wt)|nok(6|i)|nzph|o2im|op(ti|wv)|oran|owg1|p800|pan(a|d|t)|pdxg|pg(13|\-([1-8]|c))|phil|pire|pl(ay|uc)|pn\-2|po(ck|rt|se)|prox|psio|pt\-g|qa\-a|qc(07|12|21|32|60|\-[2-7]|i\-)|qtek|r380|r600|raks|rim9|ro(ve|zo)|s55\/|sa(ge|ma|mm|ms|ny|va)|sc(01|h\-|oo|p\-)|sdk\/|se(c(\-|0|1)|47|mc|nd|ri)|sgh\-|shar|sie(\-|m)|sk\-0|sl(45|id)|sm(al|ar|b3|it|t5)|so(ft|ny)|sp(01|h\-|v\-|v )|sy(01|mb)|t2(18|50)|t6(00|10|18)|ta(gt|lk)|tcl\-|tdg\-|tel(i|m)|tim\-|t\-mo|to(pl|sh)|ts(70|m\-|m3|m5)|tx\-9|up(\.b|g1|si)|utst|v400|v750|veri|vi(rg|te)|vk(40|5[0-3]|\-v)|vm40|voda|vulc|vx(52|53|60|61|70|80|81|83|85|98)|w3c(\-| )|webc|whit|wi(g |nc|nw)|wmlb|wonu|x700|yas\-|your|zeto|zte\-/i.test(ua.substr(0, 4));
return (
/(android|bb\d+|meego).+mobile|avantgo|bada\/|blackberry|blazer|compal|elaine|fennec|hiptop|iemobile|ip(hone|od)|iris|kindle|lge |maemo|midp|mmp|mobile.+firefox|netfront|opera m(ob|in)i|palm( os)?|phone|p(ixi|re)\/|plucker|pocket|psp|series(4|6)0|symbian|treo|up\.(browser|link)|vodafone|wap|windows ce|xda|xiino/i.test(
ua,
) ||
/1207|6310|6590|3gso|4thp|50[1-6]i|770s|802s|a wa|abac|ac(er|oo|s\-)|ai(ko|rn)|al(av|ca|co)|amoi|an(ex|ny|yw)|aptu|ar(ch|go)|as(te|us)|attw|au(di|\-m|r |s )|avan|be(ck|ll|nq)|bi(lb|rd)|bl(ac|az)|br(e|v)w|bumb|bw\-(n|u)|c55\/|capi|ccwa|cdm\-|cell|chtm|cldc|cmd\-|co(mp|nd)|craw|da(it|ll|ng)|dbte|dc\-s|devi|dica|dmob|do(c|p)o|ds(12|\-d)|el(49|ai)|em(l2|ul)|er(ic|k0)|esl8|ez([4-7]0|os|wa|ze)|fetc|fly(\-|_)|g1 u|g560|gene|gf\-5|g\-mo|go(\.w|od)|gr(ad|un)|haie|hcit|hd\-(m|p|t)|hei\-|hi(pt|ta)|hp( i|ip)|hs\-c|ht(c(\-| |_|a|g|p|s|t)|tp)|hu(aw|tc)|i\-(20|go|ma)|i230|iac( |\-|\/)|ibro|idea|ig01|ikom|im1k|inno|ipaq|iris|ja(t|v)a|jbro|jemu|jigs|kddi|keji|kgt( |\/)|klon|kpt |kwc\-|kyo(c|k)|le(no|xi)|lg( g|\/(k|l|u)|50|54|\-[a-w])|libw|lynx|m1\-w|m3ga|m50\/|ma(te|ui|xo)|mc(01|21|ca)|m\-cr|me(rc|ri)|mi(o8|oa|ts)|mmef|mo(01|02|bi|de|do|t(\-| |o|v)|zz)|mt(50|p1|v )|mwbp|mywa|n10[0-2]|n20[2-3]|n30(0|2)|n50(0|2|5)|n7(0(0|1)|10)|ne((c|m)\-|on|tf|wf|wg|wt)|nok(6|i)|nzph|o2im|op(ti|wv)|oran|owg1|p800|pan(a|d|t)|pdxg|pg(13|\-([1-8]|c))|phil|pire|pl(ay|uc)|pn\-2|po(ck|rt|se)|prox|psio|pt\-g|qa\-a|qc(07|12|21|32|60|\-[2-7]|i\-)|qtek|r380|r600|raks|rim9|ro(ve|zo)|s55\/|sa(ge|ma|mm|ms|ny|va)|sc(01|h\-|oo|p\-)|sdk\/|se(c(\-|0|1)|47|mc|nd|ri)|sgh\-|shar|sie(\-|m)|sk\-0|sl(45|id)|sm(al|ar|b3|it|t5)|so(ft|ny)|sp(01|h\-|v\-|v )|sy(01|mb)|t2(18|50)|t6(00|10|18)|ta(gt|lk)|tcl\-|tdg\-|tel(i|m)|tim\-|t\-mo|to(pl|sh)|ts(70|m\-|m3|m5)|tx\-9|up(\.b|g1|si)|utst|v400|v750|veri|vi(rg|te)|vk(40|5[0-3]|\-v)|vm40|voda|vulc|vx(52|53|60|61|70|80|81|83|85|98)|w3c(\-| )|webc|whit|wi(g |nc|nw)|wmlb|wonu|x700|yas\-|your|zeto|zte\-/i.test(
ua.substr(0, 4),
)
);
}

export interface UserAgentInfo {
Expand Down
25 changes: 11 additions & 14 deletions tests/api.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
/**
* @jest-environment jsdom
*/

fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
import skift from '../src/index';
import { SplitTest } from '../src/splitTest';

Expand All @@ -7,13 +11,12 @@ describe('Top-level api', () => {
expect(skift).toBeDefined();
});

it('should be impossible to create an empty test', (done) => {
it('should be impossible to create an empty test', () => {
skift.create('Awesome test!').setup()
.then(() => expect('not').toBe('here'))
.catch((err) => {
expect(skift.getTest('Awesome test!')).toBeDefined();
expect(skift.getTest('Awesome test!') instanceof SplitTest).toBe(true);
done();
});
});

Expand All @@ -25,7 +28,7 @@ describe('Top-level api', () => {
expect(test.getVariation('Variation A')).toBeDefined();
});

it('should be possible to setup a test with two variations and retrieve it by name', async (done) => {
it('should be possible to setup a test with two variations and retrieve it by name', async () => {
const test = skift
.create('Another awesome test!')
.addVariation({ name: 'Variation C' })
Expand All @@ -34,7 +37,6 @@ describe('Top-level api', () => {
expect(await test.setup()).toBe(true);
expect(skift.getTest('Another awesome test!')).toBeDefined();
expect(test === skift.getTest('Another awesome test!')).toBeTruthy();
done();
});

it('should be possible to show the UI', () => {
Expand All @@ -52,27 +54,25 @@ describe('Top-level api', () => {
});

describe('when setting a condition', () => {
it('allows for a promise', async (done) => {
it('allows for a promise', async () => {
expect(await skift
.create('testing conditions')
.setCondition(() => Promise.resolve(true))
.addVariation({ name: 'A'})
.setup()).toBe(true);
done();
});

it('allows for a boolean', async (done) => {
it('allows for a boolean', async () => {
expect(await skift
.create('testing conditions')
.setCondition(() => true)
.addVariation({ name: 'A'})
.setup()).toBe(true);
done();
});
});

describe('when checking for initialization', () => {
it('resolves to true when setup is called, completes, and was successful', async (done) => {
it('resolves to true when setup is called, completes, and was successful', async () => {
const test = skift
.create('testing conditions')
.setCondition(() => true)
Expand All @@ -83,10 +83,9 @@ describe('Top-level api', () => {

await setupPromise;
expect(await initializedPromise).toBe(true);
done();
});

it('resolves to false when setup is called, completes, and was canceled', async (done) => {
it('resolves to false when setup is called, completes, and was canceled', async () => {
const test = skift
.create('testing conditions')
.setCondition(() => false)
Expand All @@ -97,10 +96,9 @@ describe('Top-level api', () => {

await setupPromise;
expect(await initializedPromise).toBe(false);
done();
});

it('resolves to false when setup is never called', async (done) => {
it('resolves to false when setup is never called', async () => {
const test = skift
.create('testing conditions')
.setCondition(() => false)
Expand All @@ -109,7 +107,6 @@ describe('Top-level api', () => {
const initializedPromise = test.isInitialized();

expect(await initializedPromise).toBe(false);
done();
});
});
});
4 changes: 4 additions & 0 deletions tests/config.spec.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
/**
* @jest-environment jsdom
*/

import config from '../src/config';
import skift from '../src/index';

Expand Down
3 changes: 3 additions & 0 deletions tests/query.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ describe('Query', () => {

describe('#getAbTestParameter', () => {
it('should return null if there is no query', () => {
const location = {
fernicolosi marked this conversation as resolved.
Show resolved Hide resolved
search: '',
};
const abtest = query.getAbTestParameter(location.search);
expect(abtest).toBeNull();
});
Expand Down
Loading