Skip to content

Commit

Permalink
Performance improvements for parsing templates (#534)
Browse files Browse the repository at this point in the history
* Improve performance when parsing templates by caching the account details in the content script.
* Improve template context parsing and add tests for it.
* Make sure `from` is an object, and `to`, `cc`, `bcc` are arrays.
* Improve playwright test speed and accuracy.
* e2e tests for session authentication and account variable.
* Improve Outlook.com detection.
  • Loading branch information
ghinda committed Apr 3, 2024
1 parent c47a768 commit 68bbec5
Show file tree
Hide file tree
Showing 31 changed files with 756 additions and 584 deletions.
764 changes: 385 additions & 379 deletions package-lock.json

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "briskine",
"version": "7.12.4",
"version": "7.12.5",
"description": "Write everything faster.",
"private": true,
"type": "module",
Expand All @@ -13,10 +13,10 @@
"build": "webpack --mode=production --env mode=production",
"sourcebuild": "git archive --format zip --output sourcebuild/sourcebuild-${npm_package_version}.zip HEAD; zip -u sourcebuild/sourcebuild-${npm_package_version}.zip .firebase-config.json",
"lint": "eslint './**/*.js'",
"playwright:firefox": "npm run build -- manifest=2 && playwright test --project=firefox --workers=1",
"playwright:chrome": "npm run build && playwright test --project=chromium",
"playwright:firefox": "npm run build -- manifest=2 && playwright test --project=firefox",
"playwright:chrome": "npm run build && playwright test --project=chromium --workers=2",
"playwright:run": "npm run playwright:firefox && npm run playwright:chrome",
"release": "npm run lint && npm test && npm run playwright:run && npm run build -- manifest=2 && npm run build && npm run sourcebuild"
"release": "npm run lint && npm test && npm run playwright:run && npm run sourcebuild"
},
"repository": "https://github.com/briskine/briskine/",
"author": "Alexandru Plugaru <alex@gorgias.com>",
Expand Down Expand Up @@ -56,7 +56,7 @@
"mocha": "^10.0.0",
"mocha-headless-chrome": "^4.0.0",
"mockdate": "^3.0.2",
"purgecss-webpack-plugin": "^5.0.0",
"purgecss-webpack-plugin": "^6.0.0",
"web-ext": "^7.6.0",
"webpack": "^5.11.1",
"webpack-cli": "^5.0.0"
Expand Down
5 changes: 4 additions & 1 deletion playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,11 @@ export default defineConfig({
forbidOnly: false,
retries: 0,
maxFailures: 1,
workers: 2,
workers: 1,
reporter: 'line',
expect: {
timeout: 10 * 1000,
},
use: {
baseURL: baseURL,
trace: 'on-first-retry',
Expand Down
14 changes: 6 additions & 8 deletions playwright/ckeditor/ckeditor.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,19 @@ test.describe('CKEditor', () => {
const textbox = page.getByRole('textbox')
await textbox.fill('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
expect(html).toEqual('<p>Kind regards,</p><p>.</p>')
await expect(textbox).toHaveText('Kind regards,.')
})

test('should insert template from dialog', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.press('Control+ ')
const search = page.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
expect(html).toEqual('<p>It was nice talking to you.</p>')
await expect(textbox).toHaveText(template)
})
})
16 changes: 9 additions & 7 deletions playwright/ckeditor4/ckeditor4.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,33 @@ import {test, expect} from '../fixtures.ts'
test.describe('CKEditor', () => {
test.beforeEach(async ({page}) => {
await page.goto('/ckeditor4/ckeditor4.html')
// wait for briskine to load in editor frame
await page.waitForTimeout(500)
})

test.afterEach(async ({page}) => {
await page.getByRole('textbox').fill('')
})

test('should insert template with keyboard shortcut', async ({page}) => {
const textbox = page.frameLocator('.cke_wysiwyg_frame').getByRole('textbox')
const frame = page.frameLocator('.cke_wysiwyg_frame')
const textbox = frame.getByRole('textbox')
await textbox.fill('kr')
await page.waitForTimeout(500)
await textbox.press('Tab')
await expect(textbox).toHaveText('Kind regards,.')
})

test('should insert template from dialog', async ({page}) => {
const frame = page.frameLocator('.cke_wysiwyg_frame')
const textbox = frame.getByRole('textbox')
await page.waitForTimeout(500)
await textbox.press('Control+ ')
const search = frame.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = frame.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('It was nice talking to you.')
await expect(textbox).toHaveText(template)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<!doctype html>
<html>
<head>
<meta charset="utf-8">
</head>
<body>
<div style="width: 200px; height: 200px" contenteditable role="textbox"></div>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import {test, expect} from '../fixtures-auth-session.ts'

test.describe('ContentEditable Session Authenticated', () => {
test.beforeEach(async ({page}) => {
await page.goto('/contenteditable-auth-session/contenteditable-auth-session.html')
})

test.afterEach(async ({page}) => {
await page.getByRole('textbox').clear()
})

test('should insert template with keyboard shortcut', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.fill('w')
await textbox.press('Tab')
await expect(textbox).toHaveText('Write emails faster.')
})

test('should insert template from dialog', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.press('Control+ ')
const search = page.getByRole('searchbox')
await expect(search).toBeVisible()
await search.fill('create')
const template = 'Create text templates and insert them with shortcuts.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await expect(textbox).toHaveText(template)
})
})
22 changes: 18 additions & 4 deletions playwright/contenteditable-auth/contenteditable-auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,33 @@ test.describe('ContentEditable Authenticated', () => {
const textbox = page.getByRole('textbox')
await textbox.fill('w')
await textbox.press('Tab')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('Write emails faster.')
})

test('should insert template with account variable', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.fill('acc')
await textbox.press('Tab')
await expect(textbox).toHaveText('Briskine Test - Briskine Test\ncontact+test@briskine.com')
})

test('should insert template with from variable, when not available', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.fill('from')
await textbox.press('Tab')
await expect(textbox).toHaveText('Briskine Test - Briskine Test\ncontact+test@briskine.com')
})

test('should insert template from dialog', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.press('Control+ ')
const search = page.getByRole('searchbox')
await expect(search).toBeVisible()
await search.fill('create')
await page.waitForTimeout(1000)
const template = 'Create text templates and insert them with shortcuts.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('Create text templates and insert them with shortcuts.')
await expect(textbox).toHaveText(template)
})
})
14 changes: 6 additions & 8 deletions playwright/contenteditable/contenteditable.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,19 @@ test.describe('ContentEditable', () => {
const textbox = page.getByRole('textbox')
await textbox.fill('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
expect(html).toEqual('<div>Kind regards,</div><div>.</div>')
await expect(textbox).toHaveText('Kind regards,.')
})

test('should insert template from dialog', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.press('Control+ ')
const search = page.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
expect(html).toEqual('<div>It was nice talking to you.</div>')
await expect(textbox).toHaveText('It was nice talking to you.')
})
})
8 changes: 4 additions & 4 deletions playwright/draft-js/draft-js.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ test.describe('Draft.js', () => {
const textbox = page.getByRole('textbox')
await textbox.fill('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('Kind regards,.')
})

Expand All @@ -28,10 +27,11 @@ test.describe('Draft.js', () => {
await textbox.press('Control+ ')
const search = page.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('It was nice talking to you.')
})
})
24 changes: 24 additions & 0 deletions playwright/fixtures-auth-session.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/* globals process */
import dotenv from 'dotenv'

import {test} from './fixtures.ts'
export {test, expect} from './fixtures.ts'

dotenv.config()

test.skip(({browserName}) => browserName === 'firefox', 'Auth testing not supported in Firefox.')

test.beforeEach(async ({page, extensionId}) => {
await page.goto(`https://app.briskine.com/`)
await page.getByLabel('Email').fill(process.env.TEST_EMAIL || 'MISSING EMAIL')
await page.getByLabel('Password').fill(process.env.TEST_PASSWORD || 'MISSING PASSWORD')
await page.getByLabel('Password').press('Enter')
// wait for templates to load
const title = page.getByText('Write emails faster')
await title.waitFor()

await page.goto(`${extensionId}/popup/popup.html`)
await page.getByRole('button').click()
const premium = page.getByText('Go premium')
await premium.waitFor()
})
4 changes: 3 additions & 1 deletion playwright/fixtures-auth.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* globals process */
import dotenv from 'dotenv'

import {test} from './fixtures.ts'
Expand All @@ -13,5 +14,6 @@ test.beforeEach(async ({page, extensionId}) => {
await page.getByLabel('Email').fill(process.env.TEST_EMAIL || 'MISSING EMAIL')
await page.getByLabel('Password').fill(process.env.TEST_PASSWORD || 'MISSING PASSWORD')
await page.getByRole('button').click()
await page.waitForTimeout(5000)
const premium = page.getByText('Go premium')
await premium.waitFor()
})
8 changes: 4 additions & 4 deletions playwright/iframe/iframe.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ test.describe('Iframe', () => {
const textbox = page.frameLocator('#iframe').getByRole('textbox')
await textbox.fill('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('Kind regards,.')
})

Expand All @@ -23,10 +22,11 @@ test.describe('Iframe', () => {
await textbox.press('Control+ ')
const search = frame.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = frame.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('It was nice talking to you.')
})
})
8 changes: 4 additions & 4 deletions playwright/lexical/lexical.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ test.describe('Lexical', () => {
const textbox = page.getByRole('textbox')
await textbox.fill('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('Kind regards,\n.')
})

Expand All @@ -22,10 +21,11 @@ test.describe('Lexical', () => {
await textbox.press('Control+ ')
const search = page.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
await expect(textbox).toHaveText('It was nice talking to you.')
})
})
14 changes: 6 additions & 8 deletions playwright/prosemirror/prosemirror.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,19 @@ test.describe('ProseMirror', () => {
const textbox = page.getByRole('textbox')
await textbox.pressSequentially('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
const text = await textbox.innerText()
expect(text).toEqual('Kind regards,\n.')
await expect(textbox).toHaveText('Kind regards,.')
})

test('should insert template from dialog', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.press('Control+ ')
const search = page.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
expect(html).toEqual('<p>It was nice talking to you.</p>')
await expect(textbox).toHaveText('It was nice talking to you.')
})
})
16 changes: 6 additions & 10 deletions playwright/quill/quill.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,19 @@ test.describe('Quill', () => {
const textbox = page.getByRole('textbox')
await textbox.fill('kr')
await textbox.press('Tab')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
// it includes the Tab character at the end,
// since templates are not cached yet and preventDefault is not called.
expect(html).toEqual('<p>Kind regards,\n.\t</p>')
await expect(textbox).toHaveText('Kind regards,\n.')
})

test('should insert template from dialog', async ({page}) => {
const textbox = page.getByRole('textbox')
await textbox.press('Control+ ')
const search = page.getByPlaceholder('Search templates...')
await expect(search).toBeVisible()
await search.fill('nic')
await page.waitForTimeout(500)
await search.pressSequentially('nic', {delay: 100})
const template = 'It was nice talking to you.'
const list = page.getByText(template)
await list.waitFor()
await search.press('Enter')
await page.waitForTimeout(500)
const html = await textbox.innerHTML()
expect(html).toEqual('<p>It was nice talking to you.</p>')
await expect(textbox).toHaveText('It was nice talking to you.')
})
})

0 comments on commit 68bbec5

Please sign in to comment.