Skip to content

Add isLoading checking for the Example field in Create Listing Modal #4250

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.

Sign up for GitHub

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

Open
lucaslyl wants to merge 2 commits into
base: main
Choose a base branch
from
+162 −17
Open

Add isLoading checking for the Example field in Create Listing Modal #4250

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
85c5592
Fix missing examples in create listing modal and search resource sub…
lucaslyl Mar 27, 2026
8f19f26
fix lint
lucaslyl Mar 27, 2026
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
44 changes: 27 additions & 17 deletions packages/host/app/components/operator-mode/create-listing-modal.gts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,12 @@ export default class CreateListingModal extends Component<Signature> {

@tracked private selectedExamples: PickerOption[] = [];

private instancesSearch = getSearch<CardDef>(this, getOwner(this)!, () =>
this.codeRef ? { filter: { type: this.codeRef } } : undefined,
private instancesSearch = getSearch<CardDef>(
this,
getOwner(this)!,
() => (this.codeRef ? { filter: { type: this.codeRef } } : undefined),
() => (this.payload?.targetRealm ? [this.payload.targetRealm] : undefined),
{ isLive: true },
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 26, 2026

Choose a reason for hiding this comment

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

P1 Badge Avoid enabling live search without unsubscribe on close

Setting isLive: true here introduces a persistent background subscription after the modal is dismissed. CreateListingModal is always mounted (operator-mode/container.gts renders it unconditionally), and when the payload is cleared codeRef becomes undefined; in SearchResource.modify() (packages/host/app/resources/search.ts) that path returns early on query === undefined without unsubscribing existing live subscriptions or clearing #previousQuery. As a result, later realm index events can continue triggering search.perform(...) for the stale query even while the modal is closed, causing unnecessary live refresh traffic/work until the whole component is destroyed.

Useful? React with 👍 / 👎.

Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

This getSearch(..., { isLive: true }) call does not provide realms, so the search resource will treat it as “all available realms” and will subscribe live to every realm in realmServer.availableRealmURLs. That can be very expensive (many live subscriptions + refreshes) for a modal; consider constraining realms (e.g. targetRealm and/or the realm of codeRef.module) or keeping this search non-live unless you explicitly need cross-realm live updates.

Suggested change
{ isLive: true },
{ isLive: false },

Copilot uses AI. Check for mistakes.
);

private get instances(): CardDef[] {
Expand Down Expand Up @@ -119,6 +123,10 @@ export default class CreateListingModal extends Component<Signature> {
: this.initialSelected;
}

private get isCreateDisabled(): boolean {
return this.createListing.isRunning || this.instancesSearch.isLoading;
}

@action private onExampleChange(selected: PickerOption[]) {
this.selectedExamples = selected;
}
Expand Down Expand Up @@ -208,20 +216,22 @@ export default class CreateListingModal extends Component<Signature> {
</div>
</FieldContainer>

{{#if this.instanceOptions.length}}
<FieldContainer @label='Examples' class='field'>
<div class='field-contents' data-test-examples-container>
<CardInstancePicker
@placeholder='Select examples to include in the listing'
@options={{this.instanceOptions}}
@selected={{this.effectiveSelected}}
@onChange={{this.onExampleChange}}
@maxSelectedDisplay={{3}}
data-test-create-listing-examples
/>
</div>
</FieldContainer>
{{/if}}
{{#unless this.instancesSearch.isLoading}}
{{#if this.instanceOptions.length}}
<FieldContainer @label='Examples' class='field'>
Copy link
Contributor Author

@lucaslyl lucaslyl Mar 26, 2026

Choose a reason for hiding this comment

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

add loading checking and also the instance length check else the fieldcontainer will still display if the search result is empty

<div class='field-contents' data-test-examples-container>
<CardInstancePicker
@placeholder='Select examples to include in the listing'
@options={{this.instanceOptions}}
@selected={{this.effectiveSelected}}
@onChange={{this.onExampleChange}}
@maxSelectedDisplay={{3}}
data-test-create-listing-examples
/>
</div>
</FieldContainer>
{{/if}}
{{/unless}}
Comment on lines +219 to +234
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

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

The Examples picker is now hidden while instancesSearch.isLoading, but the modal’s Create button remains enabled. If the user clicks Create before the search finishes, selectedExampleURLs can resolve to an empty list (because instances/instanceOptions haven’t populated yet), resulting in a listing created without the user’s intended examples. Consider disabling Create while instancesSearch.isLoading and/or having selectedExampleURLs fall back to payload.openCardIds when options haven’t loaded yet.

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@lucaslyl lucaslyl Mar 27, 2026

Choose a reason for hiding this comment

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

make sense ,agree with this


</:content>
<:footer>
Expand Down Expand Up @@ -249,7 +259,7 @@ export default class CreateListingModal extends Component<Signature> {
@kind='primary'
@size='tall'
@loading={{this.createListing.isRunning}}
@disabled={{this.createListing.isRunning}}
@disabled={{this.isCreateDisabled}}
{{on 'click' (perform this.createListing)}}
{{onKeyMod 'Enter'}}
data-test-create-listing-confirm-button
Expand Down
6 changes: 6 additions & 0 deletions packages/host/app/resources/search.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,12 @@ export class SearchResource<
}

if (query === undefined) {
this.#previousQueryString = undefined;
this.#previousQuery = undefined;
for (let subscription of this.subscriptions) {
subscription.unsubscribe();
}
this.subscriptions = [];
return;
}

Expand Down
129 changes: 129 additions & 0 deletions packages/host/tests/integration/resources/search-test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,135 @@ module(`Integration | search resource`, function (hooks) {
);
});

test(`setting query to undefined clears cached state and re-runs search on next query`, async function (assert) {
let query: Query | undefined = {
filter: {
on: {
module: `${testRealmURL}book`,
name: 'Book',
},
eq: {
'author.lastName': 'Abdel-Rahman',
},
},
};
let args = {
query,
realms: [testRealmURL],
isLive: false,
isAutoSaved: false,
storeService,
owner: this.owner,
} satisfies SearchResourceArgs['named'];

let search = getSearchResourceForTest(loaderService, () => ({
named: args,
}));
await search.loaded;
assert.strictEqual(
search.instances.length,
2,
'initial search returns 2 books',
);

// Simulate modal close: set query to undefined
args = { ...args, query: undefined as any };
search.modify([], args);
await settled();

// Simulate modal reopen with same query
args = { ...args, query };
search.modify([], args);
await search.loaded;

assert.strictEqual(
search.instances.length,
2,
'search re-runs after query was set to undefined and back',
);
});

test(`setting query to undefined tears down live subscriptions`, async function (assert) {
let query: Query | undefined = {
filter: {
on: {
module: `${testRealmURL}book`,
name: 'Book',
},
eq: {
'author.lastName': 'Abdel-Rahman',
},
},
};
let args = {
query,
realms: [testRealmURL],
isLive: true,
isAutoSaved: false,
storeService,
owner: this.owner,
} satisfies SearchResourceArgs['named'];

let search = getSearchResourceForTest(loaderService, () => ({
named: args,
}));
await search.loaded;
assert.strictEqual(
search.instances.length,
2,
'initial live search returns 2 books',
);

// Simulate modal close: set query to undefined
args = { ...args, query: undefined as any };
search.modify([], args);
await settled();

// Write a new matching card while "modal is closed"
await realm.write(
'books/4.json',
JSON.stringify({
data: {
type: 'card',
attributes: {
author: {
firstName: 'New',
lastName: 'Abdel-Rahman',
},
editions: 0,
pubDate: '2024-01-01',
},
meta: {
adoptsFrom: {
module: `${testRealmURL}book`,
name: 'Book',
},
},
},
} as LooseSingleCardDocument),
);

await settled();

// Instances should NOT have updated since subscriptions were torn down
assert.strictEqual(
search.instances.length,
2,
'live subscription was torn down — no update while query is undefined',
);

// Simulate modal reopen: restore query
args = { ...args, query };
search.modify([], args);
await search.loaded;

assert.strictEqual(
search.instances.length,
3,
'fresh search on reopen picks up the new card',
);
});

test(`can search for file-meta instances using SearchResource`, async function (assert) {
let query: Query = {
filter: {
Expand Down
Loading