Skip to content

Add listing field with CardPill and change action to create PR modal #4287

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

Merged
tintinthong merged 2 commits into from
Apr 1, 2026
Merged

Add listing field with CardPill and change action to create PR modal #4287

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
93336f2
feat: add listing field with CardPill and change action to create PR …
lucaslyl Mar 31, 2026
2ce376c
update guard
lucaslyl Mar 31, 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
105 changes: 71 additions & 34 deletions packages/host/app/components/operator-mode/create-pr-modal.gts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,18 @@ import { service } from '@ember/service';
import Component from '@glimmer/component';
import { tracked } from '@glimmer/tracking';

import { task } from 'ember-concurrency';
import { restartableTask, task } from 'ember-concurrency';
import perform from 'ember-concurrency/helpers/perform';
import onKeyMod from 'ember-keyboard/modifiers/on-key';

import {
Button,
FieldContainer,
RealmIcon,
} from '@cardstack/boxel-ui/components';
import { Button, FieldContainer } from '@cardstack/boxel-ui/components';

import { chooseCard } from '@cardstack/runtime-common';

import CreateListingPRRequestCommand from '@cardstack/host/commands/bot-requests/create-listing-pr-request';
import CardPill from '@cardstack/host/components/card-pill';
import ModalContainer from '@cardstack/host/components/modal-container';
import { catalogRealm } from '@cardstack/host/lib/utils';

import type CommandService from '@cardstack/host/services/command-service';
import type OperatorModeStateService from '@cardstack/host/services/operator-mode-state-service';
Expand All @@ -31,6 +31,7 @@ export default class CreatePRModal extends Component<Signature> {
@service declare private realm: RealmService;

@tracked private isSubmitted = false;
@tracked private selectedListingId?: string;

private get payload() {
return this.operatorModeStateService.createPRModalPayload;
Expand All @@ -40,16 +41,19 @@ export default class CreatePRModal extends Component<Signature> {
return Boolean(this.payload);
}

private get realmInfo() {
let payload = this.payload;
if (!payload) {
private get listingId() {
return this.selectedListingId ?? this.payload?.listingId;
}

private get listingTitle(): string | undefined {
if (this.selectedListingId) {
return undefined;
}
return this.realm.info(payload.realm);
return this.payload?.listingName;
}

private get listingName(): string {
return this.payload?.listingName ?? 'Listing';
private get canChangeListing() {
return Boolean(catalogRealm);
}

private createPR = task(async () => {
Expand All @@ -58,18 +62,44 @@ export default class CreatePRModal extends Component<Signature> {
throw new Error('Cannot create PR without a modal payload');
}

let currentListingId = this.listingId;
if (!currentListingId) {
throw new Error('Cannot create PR without a listing');
}

let realm =
this.realm.realmOfURL(new URL(currentListingId))?.href ?? payload.realm;
Comment on lines +70 to +71
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

When submitting after a listing has been changed, realmOfURL(new URL(currentListingId)) can return undefined if that realm isn’t currently in realm.realms, and the code silently falls back to payload.realm. That can submit a PR request against the wrong realm. Safer options: (a) throw if the realm can’t be derived for the selected listing, or (b) derive the realm deterministically from currentListingId (instead of falling back to the original payload realm).

Suggested change
let realm =
this.realm.realmOfURL(new URL(currentListingId))?.href ?? payload.realm;
let realmURL = this.realm.realmOfURL(new URL(currentListingId));
if (!realmURL) {
throw new Error('Cannot determine realm for selected listing');
}
let realm = realmURL.href;

Copilot uses AI. Check for mistakes.

await new CreateListingPRRequestCommand(
this.commandService.commandContext,
).execute({
listingId: payload.listingId,
realm: payload.realm,
listingId: currentListingId,
realm,
});

this.isSubmitted = true;
});

private changeListing = restartableTask(async () => {
if (!catalogRealm) {
throw new Error('Cannot find catalog realm');
}
let listingId = await chooseCard({
filter: {
type: {
module: `${catalogRealm.url}catalog-app/listing/listing`,
name: 'Listing',
},
},
});
if (listingId) {
this.selectedListingId = listingId;
}
});

@action private onClose() {
this.isSubmitted = false;
this.selectedListingId = undefined;
this.operatorModeStateService.dismissCreatePRModal();
}

Expand All @@ -78,7 +108,7 @@ export default class CreatePRModal extends Component<Signature> {
<ModalContainer
class='create-pr-modal'
@cardContainerClass='create-pr'
@title={{if this.isSubmitted 'Listing Submitted 🎉 ! ' 'Make a PR'}}
@title={{if this.isSubmitted 'Listing Submitted 🎉!' 'Make a PR'}}
@size='small'
@isOpen={{this.isModalOpen}}
@onClose={{this.onClose}}
Expand All @@ -87,12 +117,10 @@ export default class CreatePRModal extends Component<Signature> {
<:content>
{{#if this.isSubmitted}}
<div class='submitted-container' data-test-create-pr-success>
<p class='submitted-message'>
Your listing
<strong>{{this.listingName}}</strong>
has been submitted for review. A PR will be created on GitHub
and you will be notified once it is approved.
</p>
<div class='submitted-message'>
Your listing has been submitted for review. A PR will be created
on GitHub and you will be notified once it is approved.
</div>
<Button
@as='anchor'
@kind='secondary'
Expand All @@ -111,15 +139,24 @@ export default class CreatePRModal extends Component<Signature> {
</p>
<FieldContainer @label='Listing' class='field'>
<div class='field-contents' data-test-create-pr-listing-name>
<span>{{this.listingName}}</span>
</div>
</FieldContainer>

<FieldContainer @label='Realm' @tag='label' class='field'>
<div class='field-contents' data-test-create-pr-realm>
{{#if this.realmInfo}}
<RealmIcon class='realm-icon' @realmInfo={{this.realmInfo}} />
<span>{{this.realmInfo.name}}</span>
{{#if this.listingId}}
<CardPill
@cardId={{this.listingId}}
@urlForRealmLookup={{this.listingId}}
@displayTitle={{this.listingTitle}}
class='listing-pill'
Comment on lines +143 to +147
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Mar 31, 2026

Choose a reason for hiding this comment

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

P2 Badge Wire listing pill click to open selected listing

The new listing field renders CardPill without an @onClick handler, so it stays non-interactive and does not navigate anywhere. In CardPill, click behavior is only enabled when @onClick is provided, so this change does not actually provide a linked listing field; users cannot open/verify the selected listing before submitting the PR.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor Author

@lucaslyl lucaslyl Mar 31, 2026

Choose a reason for hiding this comment

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

We don't want users to click to view the listing in the modal. The purpose of this field is to allow users to select a listing for submission only. So the current non-interactive behavior is correct

/>
Comment on lines +142 to +148
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The listing “pill” is rendered as a non-interactive CardPill (no @OnClick), so it doesn’t actually link/open the listing as described in the PR/issue. Consider wiring an @onClick handler (e.g. using operatorModeStateService.openCardInInteractMode(this.listingId, 'isolated')) or otherwise making the pill an anchor/button so users can navigate to the listing from the modal.

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

@lucaslyl lucaslyl Apr 1, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

  1. The CardPill is used as a label/indicator, same pattern as attachments.gts which also uses CardPill without @OnClick
  2. The modal already has a "Change" button for interaction — making the pill itself clickable would be redundant
  3. The PR modal's purpose is to create a PR, not to navigate to the listing — opening the listing would take the user away from the modal flow

{{/if}}
{{#if this.canChangeListing}}
<Button
@kind='text-only'
@size='small'
@disabled={{this.createPR.isRunning}}
{{on 'click' (perform this.changeListing)}}
data-test-create-pr-change-listing-button
>
Change
</Button>
{{/if}}
</div>
</FieldContainer>
Expand All @@ -139,8 +176,7 @@ export default class CreatePRModal extends Component<Signature> {
</Button>
{{else if this.createPR.isRunning}}
<p class='footer-loading-message' data-test-create-pr-loading>
Submitting
<strong>{{this.listingName}}</strong>. This may take a moment...
Submitting your listing. This may take a moment...
</p>
<Button
@kind='primary'
Expand Down Expand Up @@ -243,10 +279,11 @@ export default class CreatePRModal extends Component<Signature> {
.field-contents {
display: flex;
align-items: center;
justify-content: space-between;
gap: var(--horizontal-gap);
}
.realm-icon {
--boxel-realm-icon-size: 1rem;
.listing-pill :deep(figure.icon:last-child) {
display: none;
}
.footer-buttons {
display: flex;
Expand Down
80 changes: 76 additions & 4 deletions packages/host/tests/integration/components/create-pr-modal-test.gts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { waitFor } from '@ember/test-helpers';
import { click, waitFor, waitUntil } from '@ember/test-helpers';
import GlimmerComponent from '@glimmer/component';

import { module, test } from 'qunit';
Expand Down Expand Up @@ -34,7 +34,7 @@ module('Integration | components | create-pr-modal', function (hooks) {
assert.dom('[data-test-create-pr-modal]').includesText('Make a PR');
});

test('shows listing name in modal', async function (assert) {
test('shows the listing pill in modal', async function (assert) {
await renderComponent(
class TestDriver extends GlimmerComponent {
<template><OperatorMode @onClose={{noop}} /></template>
Expand All @@ -52,7 +52,7 @@ module('Integration | components | create-pr-modal', function (hooks) {
assert.dom('[data-test-create-pr-listing-name]').includesText('My Listing');
});

test('shows realm info in modal', async function (assert) {
test('does not show change action when catalog chooser is unavailable', async function (assert) {
await renderComponent(
class TestDriver extends GlimmerComponent {
<template><OperatorMode @onClose={{noop}} /></template>
Expand All @@ -67,6 +67,78 @@ module('Integration | components | create-pr-modal', function (hooks) {

await waitFor('[data-test-create-pr-modal]');

assert.dom('[data-test-create-pr-realm]').exists();
assert.dom('[data-test-create-pr-change-listing-button]').doesNotExist();
});

Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The new “Change” listing workflow isn’t covered by tests (only the absence case is asserted). Add a test that makes the chooser available (and stubs chooseCard to return a different listingId), clicks the Change button, and asserts that the pill updates and the submit action uses the newly selected listingId.

Suggested change
test('can change listing via catalog chooser and submit with new listingId', async function (assert) {
await renderComponent(
class TestDriver extends GlimmerComponent {
<template><OperatorMode @onClose={{noop}} /></template>
},
);
ctx.operatorModeStateService.showCreatePRModal({
realm: testRealmURL,
listingId: `${testRealmURL}Listing/1`,
listingName: 'My Listing',
});
// Make the catalog chooser available and stub chooseCard to return a different listing
let newListingId = `${testRealmURL}Listing/2`;
let newListingName = 'My Other Listing';
// Depending on how setupOperatorModeTests exposes the chooser, adapt this stub:
// here we assume a chooser object on the test context.
// @ts-ignore - test-time mutation
ctx.catalogChooser = ctx.catalogChooser || {};
// @ts-ignore - test-time mutation
ctx.catalogChooser.isAvailable = true;
// @ts-ignore - test-time mutation
ctx.catalogChooser.chooseCard = async () => {
return {
listingId: newListingId,
listingName: newListingName,
};
};
let submittedListingId: string | null = null;
// Stub the submit handler to capture the listingId used when submitting
// @ts-ignore - test-time mutation
ctx.operatorModeStateService.submitCreatePR = (options: { listingId: string }) => {
submittedListingId = options.listingId;
};
await waitFor('[data-test-create-pr-modal]');
// Initially shows the original listing
assert.dom('[data-test-create-pr-listing-name]').includesText('My Listing');
// Click the Change button to open the chooser and select the new listing
await click('[data-test-create-pr-change-listing-button]');
// After choosing, the pill should update to the new listing name
assert.dom('[data-test-create-pr-listing-name]').includesText(newListingName);
// Submit the PR and assert the new listingId is used
await click('[data-test-create-pr-submit-button]');
assert.strictEqual(submittedListingId, newListingId, 'submit uses newly selected listingId');
});

Copilot uses AI. Check for mistakes.
test('does not show a separate realm field in modal', async function (assert) {
await renderComponent(
class TestDriver extends GlimmerComponent {
<template><OperatorMode @onClose={{noop}} /></template>
},
);

ctx.operatorModeStateService.showCreatePRModal({
realm: testRealmURL,
listingId: `${testRealmURL}Listing/1`,
listingName: 'My Listing',
});

await waitFor('[data-test-create-pr-modal]');

assert.dom('[data-test-create-pr-realm]').doesNotExist();
});

test('cancel button dismisses the modal', async function (assert) {
await renderComponent(
class TestDriver extends GlimmerComponent {
<template><OperatorMode @onClose={{noop}} /></template>
},
);

ctx.operatorModeStateService.showCreatePRModal({
realm: testRealmURL,
listingId: `${testRealmURL}Listing/1`,
listingName: 'My Listing',
});

await waitFor('[data-test-create-pr-modal]');
assert.dom('[data-test-create-pr-modal]').exists();

await click('[data-test-create-pr-cancel-button]');

await waitUntil(
() => !document.querySelector('[data-test-create-pr-modal]'),
);
assert.dom('[data-test-create-pr-modal]').doesNotExist();
assert.strictEqual(
ctx.operatorModeStateService.createPRModalPayload,
undefined,
'modal payload is cleared after cancel',
);
});

test('submit shows success state', async function (assert) {
await renderComponent(
class TestDriver extends GlimmerComponent {
<template><OperatorMode @onClose={{noop}} /></template>
},
);

ctx.operatorModeStateService.showCreatePRModal({
realm: testRealmURL,
listingId: `${testRealmURL}Listing/1`,
listingName: 'My Listing',
});

await waitFor('[data-test-create-pr-modal]');

await click('[data-test-create-pr-confirm-button]');

await waitFor('[data-test-create-pr-success]');

assert
.dom('[data-test-create-pr-success]')
.includesText('has been submitted for review.');
assert.dom('[data-test-create-pr-done-button]').exists();
});
});
Loading