Conversation
aklkv
force-pushed
the
feat/v2
branch
3 times, most recently
from
d8b0268 to
fd70fa4
Compare
aklkv
force-pushed
the
feat/v2
branch
7 times, most recently
from
9c56234 to
cbea50f
Compare
aklkv
force-pushed
the
feat/v2
branch
13 times, most recently
from
baec30f to
8441481
Compare
aklkv
force-pushed
the
feat/v2
branch
2 times, most recently
from
7db2f9c to
cd3a2bf
Compare
aklkv
force-pushed
the
feat/v2
branch
2 times, most recently
from
5656257 to
ec1399d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades the addon to a new v2 format by removing legacy tests, introducing a TypeScript-based test-app, and adding a Kolay-powered docs-app.
- Remove all classic-format tests and test helpers
- Add a new
test-appwith TypeScript, Glint support, and modern test-loader - Introduce a
docs-apppowered by Kolay with basic rendering and route tests
Reviewed Changes
Copilot reviewed 152 out of 182 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/test-helper.js | Deleted legacy test setup |
| test-app/tests/helpers/index.ts | Added new TS test helper and wrappers around ember-qunit |
| docs-app/vite.config.mjs | Configured Vite build with Ember and Kolay plugins |
Comments suppressed due to low confidence (2)
docs-app/config/environment.js:44
- [nitpick] The production environment overrides for
rootURLandlocationTypeare commented out. If you intend to deploy this app under a subdirectory (e.g., GitHub Pages), re-enable and updateENV.rootURLandENV.locationTypeaccordingly to avoid routing issues in production.
// here you can enable a production-specific feature
test-app/tests/helpers/index.ts:13
- The
NestedHookstype is not imported in this file, which will cause a TypeScript error. Please add an import for NestedHooks from 'ember-qunit' or 'qunit' (e.g.,import type { NestedHooks } from 'ember-qunit';).
function setupApplicationTest(hooks: NestedHooks, options?: SetupTestOptions) {
|
@jkusa any update, can I do anything to get this merged and released? |
|
|
||
| function clipboard(element, params, hash) { | ||
| export interface ClipboardModifierSignature { | ||
| Args: { |
There was a problem hiding this comment.
Can we update this based on the definition here: https://github.com/jkusa/ember-cli-clipboard/blob/73790700372a540bed1213a4bcf078edcc5164d9/index.d.ts
There was a problem hiding this comment.
I tried but ClipboardJS types are bit wrong
addon/src/modifiers/clipboard.ts
Outdated
| element.setAttribute('data-clipboard-action', action); | ||
|
|
||
| if (!isBlank(text)) { | ||
| if (!isBlank(text) && text) { |
There was a problem hiding this comment.
what case does the && text catch that isBlank doesn't?
There was a problem hiding this comment.
I do not think that isBlack handles or exposes correct types to handle undefined
aklkv
force-pushed
the
feat/v2
branch
6 times, most recently
from
b018b71 to
1a3284b
Compare
|
@jkusa could we possibly merge this? |
closes: #421
What is happening here:
test-appin classic format for two things: fastboot tests and test-helpers (this is interesting one and I think we should deprecate is since helpers use private methods that are accessible in the vite https://discord.com/channels/480462759797063690/483601670685720591/1389530629921574973)Follow up items: