From 1caaf93bbfafafe8dc018adca86d93ac441cc676 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Tue, 31 May 2022 23:53:55 +0100 Subject: [PATCH 1/7] - add test selectors - add integration tests --- .../src/components/marquee.hbs | 15 ++- .../components/marquee/component-test.js | 109 ++++++++++++++++++ 2 files changed, 121 insertions(+), 3 deletions(-) create mode 100644 packages/test-app/tests/integration/components/marquee/component-test.js diff --git a/packages/ember-fast-marquee/src/components/marquee.hbs b/packages/ember-fast-marquee/src/components/marquee.hbs index 995ee3b..2d6d833 100644 --- a/packages/ember-fast-marquee/src/components/marquee.hbs +++ b/packages/ember-fast-marquee/src/components/marquee.hbs @@ -14,19 +14,28 @@ rgbaGradientColor=this.rgbaGradientColor speed=this.speed }} + data-test-marquee ...attributes > {{#if this.gradient}} -
+
{{/if}}
-
{{yield}}
+
{{yield}}
{{#each this.repeater}} - + {{/each}}
diff --git a/packages/test-app/tests/integration/components/marquee/component-test.js b/packages/test-app/tests/integration/components/marquee/component-test.js new file mode 100644 index 0000000..a68a949 --- /dev/null +++ b/packages/test-app/tests/integration/components/marquee/component-test.js @@ -0,0 +1,109 @@ +import { module, test } from 'qunit'; +import { setupRenderingTest } from 'ember-qunit'; +import { render, triggerEvent } from '@ember/test-helpers'; +import hbs from 'htmlbars-inline-precompile'; + +module('Marquee', function (hooks) { + setupRenderingTest(hooks); + + test('Renders', async function (assert) { + await render(hbs` + abc + `); + + assert.dom('[data-test-marquee-marquee]').hasText('abc'); + }); + + test('Duplicated rows are aria-hidden', async function (assert) { + await render(hbs` + abc + `); + + assert.dom('[data-test-marquee-repeater]').hasAria('hidden', 'true'); + assert.dom('[data-test-marquee-repeater]').exists({ count: 1 }); + }); + + test('@fillRow fills the row with extra duplicates', async function (assert) { + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + +
abc
+
`); + + assert.dom('[data-test-marquee-repeater]').exists({ count: 3 }); + }); + + test('@direction="left" slides left', async function (assert) { + this.set('playing', false); + await render(hbs` + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + let translateX = new DOMMatrix(scroller.transform).m41; + + assert.strictEqual(translateX, 0); + + this.set('playing', true); + await new Promise((r) => setTimeout(r, 100)); + this.set('playing', false); + + translateX = new DOMMatrix(scroller.transform).m41; + + assert.ok(translateX < 0); + }); + + test('@direction="right" slides right', async function (assert) { + this.set('playing', false); + await render(hbs` + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + let translateX = new DOMMatrix(scroller.transform).m41; + + const widthOfMarquee = this.element + .querySelector('[data-test-marquee-marquee]') + .getBoundingClientRect().width; + + assert.strictEqual( + Math.round(translateX), + -Math.abs(Math.round(widthOfMarquee)) + ); + + this.set('playing', true); + await new Promise((r) => setTimeout(r, 100)); + this.set('playing', false); + + translateX = new DOMMatrix(scroller.transform).m41; + + assert.ok(translateX < 0); + + const directionCSSVar = scroller.getPropertyValue('--direction'); + + assert.strictEqual(directionCSSVar, 'reverse'); + }); + + test('@gradient adds a gradient', async function (assert) { + await render(hbs` + abc + `); + + assert.dom('[data-test-marquee-gradient]').exists(); + }); + + test('@pauseOnHover will pause the marquee when hovered by user', async function (assert) { + await render(hbs` + abc + `); + + triggerEvent('[data-test-marquee]', 'mouseenter'); + + assert.dom('[data-test-marquee-gradient]').exists(); + }); +}); From c7a955c05ba542c565f0d12fa1b8a2ba77d70d15 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 1 Jun 2022 01:55:04 +0100 Subject: [PATCH 2/7] add tests --- .../src/components/marquee.ts | 2 +- .../src/modifiers/marquee.ts | 2 +- .../test-app/app/templates/application.hbs | 2 +- .../components/marquee/component-test.js | 173 +++++++++++++++++- 4 files changed, 172 insertions(+), 7 deletions(-) diff --git a/packages/ember-fast-marquee/src/components/marquee.ts b/packages/ember-fast-marquee/src/components/marquee.ts index 9e9502b..a425d47 100644 --- a/packages/ember-fast-marquee/src/components/marquee.ts +++ b/packages/ember-fast-marquee/src/components/marquee.ts @@ -142,7 +142,7 @@ export default class Marquee extends Component { // if we receive a string, use that as is for the value get gradientWidth(): string { const width = this.args.gradientWidth - ? this.args.gradientWidth === 'number' + ? typeof this.args.gradientWidth === 'number' ? `${this.args.gradientWidth}%` : this.args.gradientWidth : null; diff --git a/packages/ember-fast-marquee/src/modifiers/marquee.ts b/packages/ember-fast-marquee/src/modifiers/marquee.ts index d304a56..01c135e 100644 --- a/packages/ember-fast-marquee/src/modifiers/marquee.ts +++ b/packages/ember-fast-marquee/src/modifiers/marquee.ts @@ -102,7 +102,7 @@ export default class MarqueeModifier extends Modifier const duration = this.fillRow || - this.component.marqueeWidth > this.component.containerWidth + this.component.containerWidth < this.component.marqueeWidth ? this.component.marqueeWidth / this.speed : this.component.containerWidth / this.speed; diff --git a/packages/test-app/app/templates/application.hbs b/packages/test-app/app/templates/application.hbs index edb88fd..ed3ffc1 100644 --- a/packages/test-app/app/templates/application.hbs +++ b/packages/test-app/app/templates/application.hbs @@ -251,7 +251,7 @@ - +

Welcome to Ember

Welcome to the jungle

gets worse here day by day

diff --git a/packages/test-app/tests/integration/components/marquee/component-test.js b/packages/test-app/tests/integration/components/marquee/component-test.js index a68a949..5016699 100644 --- a/packages/test-app/tests/integration/components/marquee/component-test.js +++ b/packages/test-app/tests/integration/components/marquee/component-test.js @@ -1,6 +1,6 @@ import { module, test } from 'qunit'; import { setupRenderingTest } from 'ember-qunit'; -import { render, triggerEvent } from '@ember/test-helpers'; +import { render } from '@ember/test-helpers'; import hbs from 'htmlbars-inline-precompile'; module('Marquee', function (hooks) { @@ -14,6 +14,26 @@ module('Marquee', function (hooks) { assert.dom('[data-test-marquee-marquee]').hasText('abc'); }); + test('Play state can be toggled', async function (assert) { + this.set('play', false); + await render(hbs` + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + let playState = scroller.getPropertyValue('animation-play-state'); + + assert.strictEqual(playState, 'paused'); + this.set('play', true); + + playState = scroller.getPropertyValue('animation-play-state'); + + assert.strictEqual(playState, 'running'); + }); + test('Duplicated rows are aria-hidden', async function (assert) { await render(hbs` abc @@ -25,7 +45,7 @@ module('Marquee', function (hooks) { test('@fillRow fills the row with extra duplicates', async function (assert) { await render(hbs`{{!-- template-lint-disable no-inline-styles --}} - +
abc
`); @@ -102,8 +122,153 @@ module('Marquee', function (hooks) { abc
`); - triggerEvent('[data-test-marquee]', 'mouseenter'); + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); - assert.dom('[data-test-marquee-gradient]').exists(); + // if there is a bettr way to handle :hover selector testing + // then this should be updated. For now we can only check the + // css variable is correctly set + assert.strictEqual(scroller.getPropertyValue('--pause-on-hover'), 'paused'); + }); + + test('@pauseOnClick will pause the marquee when hovered by user', async function (assert) { + await render(hbs` + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + // as above but for :active + assert.strictEqual(scroller.getPropertyValue('--pause-on-click'), 'paused'); + }); + + test('@onCycleComplete fires', async function (assert) { + assert.expect(1); + const done = assert.async(); + const action = () => { + assert.ok(true); + done(); + }; + this.set('action', action); + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + }); + + test('@onFinish fires', async function (assert) { + assert.expect(2); + const done = assert.async(); + + const action = () => { + assert.ok(true); + done(); + }; + + this.set('action', action); + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + assert.strictEqual(scroller.getPropertyValue('--iteration-count'), '1'); + }); + + test('@loop > 0 results in an equal iteration-count', async function (assert) { + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + assert.strictEqual(scroller.getPropertyValue('--iteration-count'), '2'); + }); + + test('@speed 20 creates the correct animation duration for width 400px', async function (assert) { + this.set('speed', 20); + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + + // A container of 400px will have 2 content boxes each 400px each + // resulting in a scroll container 800px wide. + // + // [ 800px ] + // [ 400px ] [ 400px ] + // ------------------------------------------------------------- + // | | | + // ------------------------------------------------------------- + // + // The scroll container will be moved 400px to the left to complete a cycle + // width / speed = duration in seconds + // 400/20 = 20 + // + // We need to account for the scale(0.5) applied to the #ember-testing container + // so instead of expecting 20s we expect 10s as all our widths are halfed since + // we use getBoundingClientRect().width to find the onscreen width of the marquee + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + assert.strictEqual(scroller.getPropertyValue('--duration'), '10s'); + }); + + test('@delay > 0 results in an equal delay variable', async function (assert) { + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + assert.strictEqual(scroller.getPropertyValue('--delay'), '1s'); + }); + + test('@gradientColor is properly formatted', async function (assert) { + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + assert.strictEqual( + scroller.getPropertyValue('--gradient-color'), + 'rgba(255, 0, 0, 1), rgba(255, 0, 0, 0)' + ); + }); + + test('@gradientWidth can be either number or string', async function (assert) { + this.set('gradientWidth', 10); + await render(hbs`{{!-- template-lint-disable no-inline-styles --}} + + abc + `); + + const scroller = getComputedStyle( + this.element.querySelector('[data-test-marquee-scroller]') + ); + + assert.strictEqual(scroller.getPropertyValue('--gradient-width'), '10%'); + + this.set('gradientWidth', '10px'); + + assert.strictEqual(scroller.getPropertyValue('--gradient-width'), '10px'); }); }); From 713f18e5780796ea655a89ca965b65eec05a3bc4 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 1 Jun 2022 02:32:02 +0100 Subject: [PATCH 3/7] Remove data-test selectors from published package --- package.json | 2 +- packages/ember-fast-marquee/babel.config.js | 30 +++++++++++++++++++ packages/ember-fast-marquee/babel.config.json | 8 ----- packages/ember-fast-marquee/package.json | 1 + yarn.lock | 5 ++++ 5 files changed, 37 insertions(+), 9 deletions(-) create mode 100644 packages/ember-fast-marquee/babel.config.js delete mode 100644 packages/ember-fast-marquee/babel.config.json diff --git a/package.json b/package.json index abfc40d..315b14d 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ ], "scripts": { "prepare": "yarn workspace ember-fast-marquee run prepare", - "start": "npm-run-all --parallel start:*", + "start": "KEEP_DATA_TEST_SELECTORS=1 npm-run-all --parallel start:*", "start:addon": "yarn workspace ember-fast-marquee run start", "start:test-app": "yarn workspace test-app run start", "lint": "npm-run-all --aggregate-output --continue-on-error --parallel \"lint:!(fix)\"", diff --git a/packages/ember-fast-marquee/babel.config.js b/packages/ember-fast-marquee/babel.config.js new file mode 100644 index 0000000..e2bb3df --- /dev/null +++ b/packages/ember-fast-marquee/babel.config.js @@ -0,0 +1,30 @@ +'use strict'; +/* eslint-env node */ + +const keepSelectors = + process.env.KEEP_DATA_TEST_SELECTORS === '1' ? true : false; + +const babelConfig = { + presets: [['@babel/preset-typescript']], + plugins: [ + ['@babel/plugin-proposal-decorators', { legacy: true }], + '@babel/plugin-proposal-class-properties', + '@embroider/addon-dev/template-colocation-plugin', + ], +}; + +if (!keepSelectors) { + babelConfig.plugins.push([ + 'search-and-replace', + { + rules: [ + { + search: /data-test[a-zA-Z-]*/g, + replace: '', + }, + ], + }, + ]); +} + +module.exports = babelConfig; diff --git a/packages/ember-fast-marquee/babel.config.json b/packages/ember-fast-marquee/babel.config.json deleted file mode 100644 index fe782c2..0000000 --- a/packages/ember-fast-marquee/babel.config.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "presets": [["@babel/preset-typescript"]], - "plugins": [ - ["@babel/plugin-proposal-decorators", { "legacy": true }], - "@babel/plugin-proposal-class-properties", - "@embroider/addon-dev/template-colocation-plugin" - ] -} diff --git a/packages/ember-fast-marquee/package.json b/packages/ember-fast-marquee/package.json index 51d3127..8004057 100644 --- a/packages/ember-fast-marquee/package.json +++ b/packages/ember-fast-marquee/package.json @@ -49,6 +49,7 @@ "@typescript-eslint/parser": "^5.26.0", "autoprefixer": "^10.4.7", "babel-eslint": "^10.1.0", + "babel-plugin-search-and-replace": "^1.1.0", "ember-template-lint": "^4.9.1", "eslint": "^7.32.0", "eslint-config-prettier": "^8.5.0", diff --git a/yarn.lock b/yarn.lock index 60f98cb..f8f4c14 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3322,6 +3322,11 @@ babel-plugin-polyfill-regenerator@^0.3.0: dependencies: "@babel/helper-define-polyfill-provider" "^0.3.1" +babel-plugin-search-and-replace@^1.1.0: + version "1.1.0" + resolved "https://registry.yarnpkg.com/babel-plugin-search-and-replace/-/babel-plugin-search-and-replace-1.1.0.tgz#ad26f2037a80034613dae61b913902d9aa58ed2c" + integrity sha512-qo3E08GmT43/IgfbEiAZoauYW3SILLTyt79QPZc2ME3sHvuBuRA01nQl7WnJRKZTnqUCimXSMOU+LBHxe+HTUw== + babel-plugin-syntax-dynamic-import@^6.18.0: version "6.18.0" resolved "https://registry.yarnpkg.com/babel-plugin-syntax-dynamic-import/-/babel-plugin-syntax-dynamic-import-6.18.0.tgz#8d6a26229c83745a9982a441051572caa179b1da" From 1d28c46345ce37867144a682fc35c9d0c0ce26db Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 1 Jun 2022 02:51:18 +0100 Subject: [PATCH 4/7] try moving to build scripts instead of prepare --- .github/workflows/ci.yml | 9 ++++----- package.json | 6 ++++-- packages/ember-fast-marquee/babel.config.js | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 251693e..b12081f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -8,8 +8,8 @@ on: pull_request: {} concurrency: - group: ci-${{ github.head_ref || github.ref }} - cancel-in-progress: true + group: ci-${{ github.head_ref || github.ref }} + cancel-in-progress: true jobs: test: @@ -28,7 +28,7 @@ jobs: - name: Lint run: yarn lint - name: Test - run: yarn test + run: yarn buildForTest && yarn test test-try: name: Additional Tests @@ -57,6 +57,5 @@ jobs: - name: Install dependencies uses: bahmutov/npm-install@v1 - name: Test - run: yarn ember try:one ${{ matrix.scenario }} + run: yarn buildForTest && yarn ember try:one ${{ matrix.scenario }} working-directory: packages/test-app - diff --git a/package.json b/package.json index 315b14d..88327be 100644 --- a/package.json +++ b/package.json @@ -8,7 +8,8 @@ "packages/*" ], "scripts": { - "prepare": "yarn workspace ember-fast-marquee run prepare", + "build": "yarn workspace ember-fast-marquee run prepare", + "buildForTest": "KEEP_DATA_TEST_SELECTORS=1 yarn workspace ember-fast-marquee run prepare", "start": "KEEP_DATA_TEST_SELECTORS=1 npm-run-all --parallel start:*", "start:addon": "yarn workspace ember-fast-marquee run start", "start:test-app": "yarn workspace test-app run start", @@ -22,7 +23,8 @@ "test:watch": "npm-run-all --aggregate-output --continue-on-error --parallel test:watch:*", "test:test-app": "yarn workspace test-app run test", "test:watch:test-app": "yarn workspace test-app run test:watch", - "test:watch:addon": "yarn workspace ember-fast-marquee run start" + "test:watch:addon": "yarn workspace ember-fast-marquee run start", + "release": "yarn build && release-it" }, "devDependencies": { "npm-run-all": "^4.1.5", diff --git a/packages/ember-fast-marquee/babel.config.js b/packages/ember-fast-marquee/babel.config.js index e2bb3df..1078072 100644 --- a/packages/ember-fast-marquee/babel.config.js +++ b/packages/ember-fast-marquee/babel.config.js @@ -19,7 +19,7 @@ if (!keepSelectors) { { rules: [ { - search: /data-test[a-zA-Z-]*/g, + search: /data-test[a-zA-Z0-9-]*/g, replace: '', }, ], From 3821e4930bc7ee46a7d94e437aa90928c7904815 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 1 Jun 2022 02:55:23 +0100 Subject: [PATCH 5/7] remove buildForTest from ember try scenarios --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b12081f..1270311 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,5 +57,5 @@ jobs: - name: Install dependencies uses: bahmutov/npm-install@v1 - name: Test - run: yarn buildForTest && yarn ember try:one ${{ matrix.scenario }} + run: yarn ember try:one ${{ matrix.scenario }} working-directory: packages/test-app From 5afac1d1afd1a95d5d3e77658dfe9910ac227873 Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 1 Jun 2022 03:02:24 +0100 Subject: [PATCH 6/7] try a postinstall hook --- .github/workflows/ci.yml | 2 +- package.json | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1270311..d27297d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,5 +57,5 @@ jobs: - name: Install dependencies uses: bahmutov/npm-install@v1 - name: Test - run: yarn ember try:one ${{ matrix.scenario }} + run: KEEP_DATA_TEST_SELECTORS=1 yarn ember try:one ${{ matrix.scenario }} working-directory: packages/test-app diff --git a/package.json b/package.json index 88327be..2b410b4 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ "packages/*" ], "scripts": { + "postintall": "yarn build", "build": "yarn workspace ember-fast-marquee run prepare", "buildForTest": "KEEP_DATA_TEST_SELECTORS=1 yarn workspace ember-fast-marquee run prepare", "start": "KEEP_DATA_TEST_SELECTORS=1 npm-run-all --parallel start:*", From 96e9c60eed7911ad6d250df815257e4efa03695e Mon Sep 17 00:00:00 2001 From: Liam Potter Date: Wed, 1 Jun 2022 03:06:15 +0100 Subject: [PATCH 7/7] try using an env var in the ci config --- .github/workflows/ci.yml | 7 +++++-- package.json | 9 +++------ 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d27297d..afe2f64 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -7,6 +7,9 @@ on: - master pull_request: {} +env: + KEEP_DATA_TEST_SELECTORS: '1' + concurrency: group: ci-${{ github.head_ref || github.ref }} cancel-in-progress: true @@ -28,7 +31,7 @@ jobs: - name: Lint run: yarn lint - name: Test - run: yarn buildForTest && yarn test + run: yarn test test-try: name: Additional Tests @@ -57,5 +60,5 @@ jobs: - name: Install dependencies uses: bahmutov/npm-install@v1 - name: Test - run: KEEP_DATA_TEST_SELECTORS=1 yarn ember try:one ${{ matrix.scenario }} + run: yarn ember try:one ${{ matrix.scenario }} working-directory: packages/test-app diff --git a/package.json b/package.json index 2b410b4..1200bbd 100644 --- a/package.json +++ b/package.json @@ -8,10 +8,8 @@ "packages/*" ], "scripts": { - "postintall": "yarn build", - "build": "yarn workspace ember-fast-marquee run prepare", - "buildForTest": "KEEP_DATA_TEST_SELECTORS=1 yarn workspace ember-fast-marquee run prepare", - "start": "KEEP_DATA_TEST_SELECTORS=1 npm-run-all --parallel start:*", + "prepare": "yarn workspace ember-fast-marquee run prepare", + "start": "KEEP_DATA_TEST_SELECOTRS=1 npm-run-all --parallel start:*", "start:addon": "yarn workspace ember-fast-marquee run start", "start:test-app": "yarn workspace test-app run start", "lint": "npm-run-all --aggregate-output --continue-on-error --parallel \"lint:!(fix)\"", @@ -24,8 +22,7 @@ "test:watch": "npm-run-all --aggregate-output --continue-on-error --parallel test:watch:*", "test:test-app": "yarn workspace test-app run test", "test:watch:test-app": "yarn workspace test-app run test:watch", - "test:watch:addon": "yarn workspace ember-fast-marquee run start", - "release": "yarn build && release-it" + "test:watch:addon": "yarn workspace ember-fast-marquee run start" }, "devDependencies": { "npm-run-all": "^4.1.5",