Skip to content

feat: v2 - #422

Closed
aklkv wants to merge 0 commit into
jkusa:nextfrom
adopted-ember-addons:feat/v2
Closed

feat: v2#422
aklkv wants to merge 0 commit into
jkusa:nextfrom
adopted-ember-addons:feat/v2

Conversation

@aklkv

@aklkv aklkv commented Oct 23, 2024

Copy link
Copy Markdown
Contributor

closes: #421

What is happening here:

  • addon is not new v2 format with it's own tests + gts and generated type declarations
  • added new docs-app that os powered by kolay there is a basic tests that make sure that all pages are rendered with docs content
  • I left old test-app in 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:

  • maybe we release this is a major with test-stupport in place and we deprecate test-support helpers, and in the next major we can drop this test helpers
  • setup release-plan
  • setup deployment of docs-app

@aklkv
aklkv force-pushed the feat/v2 branch 3 times, most recently from d8b0268 to fd70fa4 Compare October 23, 2024 08:02
@aklkv
aklkv force-pushed the feat/v2 branch 7 times, most recently from 9c56234 to cbea50f Compare January 27, 2025 08:37
@aklkv
aklkv marked this pull request as ready for review January 27, 2025 08:41
@aklkv
aklkv force-pushed the feat/v2 branch 13 times, most recently from baec30f to 8441481 Compare July 3, 2025 02:07
@aklkv
aklkv force-pushed the feat/v2 branch 2 times, most recently from 7db2f9c to cd3a2bf Compare July 6, 2025 10:44
@jkusa
jkusa requested a review from Copilot July 12, 2025 15:57
@jkusa jkusa self-assigned this Jul 12, 2025

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

@aklkv
aklkv force-pushed the feat/v2 branch 2 times, most recently from 5656257 to ec1399d Compare July 13, 2025 00:43
@aklkv
aklkv requested a review from Copilot July 13, 2025 00:44

This comment was marked as outdated.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-app with TypeScript, Glint support, and modern test-loader
  • Introduce a docs-app powered 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 rootURL and locationType are commented out. If you intend to deploy this app under a subdirectory (e.g., GitHub Pages), re-enable and update ENV.rootURL and ENV.locationType accordingly to avoid routing issues in production.
    // here you can enable a production-specific feature

test-app/tests/helpers/index.ts:13

  • The NestedHooks type 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) {

@aklkv

aklkv commented Jul 25, 2025

Copy link
Copy Markdown
Contributor Author

@jkusa any update, can I do anything to get this merged and released?

@jkusa jkusa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @aklkv for this PR 🙏 Left a few comments. Also, I would like to rev the doc site a bit, but want to get this PR landed. Can you please switch the PR target to next?

Comment thread CONTRIBUTING.md Outdated
Comment thread README.md Outdated
Comment thread README.md
Comment thread addon/src/modifiers/clipboard.ts Outdated

function clipboard(element, params, hash) {
export interface ClipboardModifierSignature {
Args: {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried but ClipboardJS types are bit wrong

Screenshot 2025-07-31 at 8 33 05 PM Screenshot 2025-07-31 at 8 33 26 PM

Comment thread addon/src/modifiers/clipboard.ts Outdated
element.setAttribute('data-clipboard-action', action);

if (!isBlank(text)) {
if (!isBlank(text) && text) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what case does the && text catch that isBlank doesn't?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not think that isBlack handles or exposes correct types to handle undefined

Screenshot 2025-07-31 at 8 34 36 PM

Comment thread addon/src/modifiers/clipboard.ts Outdated
@aklkv
aklkv changed the base branch from main to next August 1, 2025 02:35
@aklkv
aklkv force-pushed the feat/v2 branch 2 times, most recently from cefbc13 to 76ecc2e Compare August 1, 2025 03:48
@aklkv
aklkv requested a review from jkusa August 1, 2025 03:50
@aklkv
aklkv force-pushed the feat/v2 branch 7 times, most recently from 1a3284b to 9a95d52 Compare August 1, 2025 05:02
@aklkv

aklkv commented Oct 4, 2025

Copy link
Copy Markdown
Contributor Author

@jkusa could we possibly merge this?

@aklkv

aklkv commented Apr 16, 2026

Copy link
Copy Markdown
Contributor Author

@jkusa 👋🏻 any chance we can move this along?

@aklkv
aklkv force-pushed the feat/v2 branch 2 times, most recently from 0ae6df7 to eecf79b Compare May 29, 2026 23:50
@aklkv aklkv closed this May 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v2

3 participants