chore: hide GitHub feedback forms if there are no GitHub feedback links (#17062)

* chore: hide GitHub feedback forms if there are no GitHub feedback links
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: check for GitHub bug or feature links
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: fix AI suggestions and add test
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: add feedbackLinks in product.json
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: fix tests
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: apply reviews
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: apply reviews
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: fix tests
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: fix tests
Signed-off-by: Sonia Sandler <ssandler@redhat.com>

* chore: apply comments
Signed-off-by: Sonia Sandler <ssandler@redhat.com>
This commit is contained in:
Sonia Sandler 2026-04-19 23:05:34 -04:00 committed by GitHub
parent 0a6b176787
commit 8227f59c2a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 277 additions and 30 deletions

View file

@ -18,8 +18,9 @@
export type DirectFeedbackCategory = 'developers' | 'design';
export type GitHubFeedbackCategory = 'feature' | 'bug';
export type OtherFeedbackCategory = 'other';
export type FeedbackCategory = DirectFeedbackCategory | GitHubFeedbackCategory;
export type FeedbackCategory = DirectFeedbackCategory | GitHubFeedbackCategory | OtherFeedbackCategory;
export interface FeedbackProperties {
category: FeedbackCategory;
@ -29,7 +30,7 @@ export interface FeedbackProperties {
}
export interface GitHubIssue {
category: FeedbackCategory;
category: GitHubFeedbackCategory;
title: string;
description: string;
includeSystemInfo?: boolean;

View file

@ -126,6 +126,9 @@ describe('openGitHubIssue', () => {
await feedbackHandler.openGitHubIssue(issueProperties);
expect(shell.openExternal).toHaveBeenCalledOnce();
expect(shell.openExternal).toHaveBeenCalledWith(
expect.stringContaining(productJSONFile.GitHubFeedbackLinks.issues),
);
// extract the first argument of the shell.openExternal call
const url: string | undefined = vi.mocked(shell.openExternal).mock.calls[0]?.[0];
@ -136,6 +139,30 @@ describe('openGitHubIssue', () => {
});
});
test('Expect log in the console is there is no GitHub issues link for preview', async () => {
const originalConsoleLog = console.log;
const consoleLogMock = vi.fn();
console.log = consoleLogMock;
const originalLink = vi.mocked(productJSONFile).GitHubFeedbackLinks.issues;
vi.mocked(productJSONFile).GitHubFeedbackLinks.issues = '';
const issueProperties: GitHubIssue = {
category: 'feature',
title: 'new feature',
description: 'feature description',
};
const feedbackHandler = new FeedbackHandler(extensionLoaderMock);
await feedbackHandler.openGitHubIssue(issueProperties);
expect(consoleLogMock).toHaveBeenCalledWith('No GitHub issues link found, cannot preview new GitHub issue');
console.log = originalConsoleLog;
vi.mocked(productJSONFile).GitHubFeedbackLinks.issues = originalLink;
});
test('Expect openExternal to be called with queryParams and feature request template', async () => {
const issueProperties: GitHubIssue = {
category: 'feature',

View file

@ -43,15 +43,29 @@ export class FeedbackHandler {
}
async openGitHubIssue(issueProperties: GitHubIssue): Promise<void> {
let additionalContent: string | undefined;
if (issueProperties.includeExtensionInfo) {
const extensions = await this.getStartedExtensions();
additionalContent = `**Enabled Extensions**\n${extensions}`;
}
if (productJSONFile.GitHubFeedbackLinks?.issues) {
let additionalContent: string | undefined;
if (issueProperties.includeExtensionInfo) {
const extensions = await this.getStartedExtensions();
additionalContent = `**Enabled Extensions**\n${extensions}`;
}
const urlSearchParams = new URLSearchParams(this.toQueryParameters(issueProperties, additionalContent)).toString();
const link = `https://github.com/containers/podman-desktop/issues/new?${urlSearchParams}`;
await shell.openExternal(link);
const urlSearchParams = new URLSearchParams(
this.toQueryParameters(issueProperties, additionalContent),
).toString();
const link = `${productJSONFile.GitHubFeedbackLinks?.issues}/new?${urlSearchParams}`;
await shell.openExternal(link);
} else {
console.log('No GitHub issues link found, cannot preview new GitHub issue');
}
}
getGitHubFeedbackLinks(): { [category: string]: string } | undefined {
return productJSONFile.GitHubFeedbackLinks;
}
getFeedbackLinks(): { [category: string]: string } | undefined {
return productJSONFile.feedbackLinks;
}
protected async getStartedExtensions(): Promise<string> {

View file

@ -3079,6 +3079,20 @@ export class PluginSystem {
return feedback.openGitHubIssue(properties);
});
this.ipcHandle(
'feedback:getGitHubFeedbackLinks',
async (_listener): Promise<{ [category: string]: string } | undefined> => {
return feedback.getGitHubFeedbackLinks();
},
);
this.ipcHandle(
'feedback:getFeedbackLinks',
async (_listener): Promise<{ [category: string]: string } | undefined> => {
return feedback.getFeedbackLinks();
},
);
this.ipcHandle('feedback:getFeedbackMessages', async (): Promise<FeedbackMessages> => {
return feedback.getFeedbackMessages();
});

View file

@ -2444,6 +2444,17 @@ export function initExposure(): void {
return ipcInvoke('feedback:GitHubPreview', feedback);
});
contextBridge.exposeInMainWorld(
'getGitHubFeedbackLinks',
async (): Promise<{ [category: string]: string } | undefined> => {
return ipcInvoke('feedback:getGitHubFeedbackLinks');
},
);
contextBridge.exposeInMainWorld('getFeedbackLinks', async (): Promise<{ [category: string]: string } | undefined> => {
return ipcInvoke('feedback:getFeedbackLinks');
});
contextBridge.exposeInMainWorld('getFeedbackMessages', async (): Promise<FeedbackMessages> => {
return ipcInvoke('feedback:getFeedbackMessages');
});

View file

@ -44,11 +44,17 @@ beforeAll(() => {
beforeEach(() => {
vi.resetAllMocks();
vi.mocked(window.getGitHubFeedbackLinks).mockResolvedValue({
bug: '/bug/link',
feature: '/feature/link',
});
});
test('Expect developers feedback form to be rendered by default', () => {
test('Expect developers feedback form to be rendered by default', async () => {
render(SendFeedback);
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
expect(DirectFeedback).toHaveBeenCalledOnce();
expect(DirectFeedback).toHaveBeenCalledWith(expect.anything(), {
onCloseForm: expect.any(Function),
@ -61,6 +67,8 @@ test('Expect developers feedback form to be rendered by default', () => {
test('Expect confirmation dialog to be displayed if content changed', async () => {
render(SendFeedback, {});
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
expect(DirectFeedback).toHaveBeenCalledWith(expect.anything(), {
onCloseForm: expect.any(Function),
category: 'developers',
@ -87,6 +95,8 @@ test('Expect confirmation dialog to be displayed if content changed', async () =
test('Expect no confirmation dialog to be displayed if content has not changed', async () => {
render(SendFeedback, {});
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
expect(DirectFeedback).toHaveBeenCalledWith(expect.anything(), {
onCloseForm: expect.any(Function),
category: 'developers',
@ -105,6 +115,8 @@ test('Expect no confirmation dialog to be displayed if content has not changed',
test('Expect DirectFeedback form to be rendered when design category is selected', async () => {
render(SendFeedback, {});
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ });
expect(categorySelect).toBeInTheDocument();
categorySelect.focus();
@ -126,6 +138,8 @@ test('Expect DirectFeedback form to be rendered when design category is selected
test('Expect GitHubIssue feedback form to be rendered if category is not developers', async () => {
render(SendFeedback, {});
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ });
expect(categorySelect).toBeInTheDocument();
categorySelect.focus();
@ -138,6 +152,10 @@ test('Expect GitHubIssue feedback form to be rendered if category is not develop
// click on a smiley
expect(vi.mocked(GitHubIssueFeedback)).toHaveBeenNthCalledWith(1, expect.anything(), {
onCloseForm: expect.any(Function),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
category: 'bug',
contentChange: expect.any(Function),
});
@ -150,7 +168,71 @@ test('Expect GitHubIssue feedback form to be rendered if category is not develop
await fireEvent.click(featureCategory);
expect(vi.mocked(GitHubIssueFeedback)).toHaveBeenNthCalledWith(1, expect.anything(), {
onCloseForm: expect.any(Function),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
category: 'feature',
contentChange: expect.any(Function),
});
});
test('Expect if there no GitHub links to have a new category other with other feedback links', async () => {
vi.mocked(window.getGitHubFeedbackLinks).mockResolvedValue(undefined);
vi.mocked(window.getFeedbackLinks).mockResolvedValue({
category1: '/catgory1/link',
category2: '/catgory2/link',
category3: '/catgory3/link',
});
render(SendFeedback);
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ });
expect(categorySelect).toBeInTheDocument();
categorySelect.focus();
// open feedback categories dropdown
await userEvent.keyboard('[ArrowDown]');
const bugCategory = screen.queryByRole('button', { name: /Bug/ });
expect(bugCategory).not.toBeInTheDocument();
const featureCategory = screen.queryByRole('button', { name: /Feature/ });
expect(featureCategory).not.toBeInTheDocument();
const otherCategory = screen.getByRole('button', { name: /Other/ });
expect(otherCategory).toBeInTheDocument();
await fireEvent.click(otherCategory);
expect(screen.getByText('category1')).toBeInTheDocument();
expect(screen.getByText('category2')).toBeInTheDocument();
expect(screen.getByText('category3')).toBeInTheDocument();
});
test('Expect if there no GitHub links to not have a new category other if there are also no other feedback links', async () => {
vi.mocked(window.getGitHubFeedbackLinks).mockResolvedValue(undefined);
vi.mocked(window.getFeedbackLinks).mockResolvedValue({});
render(SendFeedback);
await vi.waitFor(() => expect(window.getGitHubFeedbackLinks).toHaveBeenCalled());
const categorySelect = screen.getByRole('button', { name: /Direct your words to the developers/ });
expect(categorySelect).toBeInTheDocument();
categorySelect.focus();
// open feedback categories dropdown
await userEvent.keyboard('[ArrowDown]');
const bugCategory = screen.queryByRole('button', { name: /Bug/ });
expect(bugCategory).not.toBeInTheDocument();
const featureCategory = screen.queryByRole('button', { name: /Feature/ });
expect(featureCategory).not.toBeInTheDocument();
const otherCategory = screen.queryByRole('button', { name: /Other/ });
expect(otherCategory).not.toBeInTheDocument();
});

View file

@ -1,6 +1,10 @@
<script lang="ts">
import type { FeedbackCategory } from '@podman-desktop/core-api';
import { CloseButton, Dropdown, Modal } from '@podman-desktop/ui-svelte';
import { Button, CloseButton, Dropdown, Link, Modal } from '@podman-desktop/ui-svelte';
import { onMount } from 'svelte';
import { SvelteMap } from 'svelte/reactivity';
import FeedbackForm from '/@/lib/feedback/FeedbackForm.svelte';
import DirectFeedback from './feedbackForms/DirectFeedback.svelte';
import GitHubIssueFeedback from './feedbackForms/GitHubIssueFeedback.svelte';
@ -9,12 +13,12 @@ let displayModal = $state(false);
const DEFAULT_CATEGORY: FeedbackCategory = 'developers';
let category: FeedbackCategory = $state(DEFAULT_CATEGORY);
let hasContent: boolean = false;
let categoryGitHubLinks: { [category: string]: string } | undefined = $state({});
let feedbackLinks: { [category: string]: string } = $state({});
const FEEDBACK_CATEGORIES = new Map<FeedbackCategory, string>([
const feedbackCategories = new SvelteMap<FeedbackCategory, string>([
['developers', '💬 Direct your words to the developers'],
['design', '🎨 User experience or design thoughts'],
['feature', '🚀 Feature request'],
['bug', '🪲 Bug'],
]);
window.events?.receive('display-feedback', () => {
@ -52,6 +56,23 @@ async function hideModal(confirm = true): Promise<void> {
function handleUpdate(e: boolean): void {
hasContent = e;
}
onMount(async () => {
categoryGitHubLinks = await window.getGitHubFeedbackLinks();
if (categoryGitHubLinks && (categoryGitHubLinks.feature || categoryGitHubLinks.bug)) {
if (categoryGitHubLinks.feature) {
feedbackCategories.set('feature', '🚀 Feature request');
}
if (categoryGitHubLinks.bug) {
feedbackCategories.set('bug', '🪲 Bug');
}
} else {
feedbackLinks = (await window.getFeedbackLinks()) ?? {};
if (Object.keys(feedbackLinks).length > 0) {
feedbackCategories.set('other', '❓ Other');
}
}
});
</script>
{#if displayModal}
@ -65,14 +86,26 @@ function handleUpdate(e: boolean): void {
<div class="relative text-[var(--pd-modal-text)] px-10 pt-4">
<label for="category" class="block mb-2 text-sm font-medium text-[var(--pd-modal-text)]">Category</label>
<Dropdown name="category" bind:value={category}
options={Array.from(FEEDBACK_CATEGORIES).map(e => ({ value: e[0], label: e[1] }))}>
options={Array.from(feedbackCategories).map(e => ({ value: e[0], label: e[1] }))}>
</Dropdown>
</div>
{#if category === 'developers' || category === 'design'}
<DirectFeedback onCloseForm={hideModal} category={category} contentChange={handleUpdate}/>
{:else if category === 'bug' || category === 'feature'}
<GitHubIssueFeedback onCloseForm={hideModal} category={category} contentChange={handleUpdate}/>
<GitHubIssueFeedback onCloseForm={hideModal} category={category} categoryLinks={categoryGitHubLinks} contentChange={handleUpdate}/>
{:else if category === 'other'}
<FeedbackForm>
<svelte:fragment slot="content">
<p class="block mt-4 mb-4 text-sm font-medium text-[var(--pd-modal-text)]">Could not find the right category? Take a look at these additional options:</p>
{#each Object.entries(feedbackLinks) as [category, link] (category)}
<Link aria-label={`${category} link`} class="block mt-1" onclick={(): Promise<void> => window.openExternal(link)}>{category}</Link>
{/each}
</svelte:fragment>
<svelte:fragment slot="buttons">
<Button class="underline" type="link" aria-label="Cancel" onclick={hideModal}>Cancel</Button>
</svelte:fragment>
</FeedbackForm>
{/if}
</Modal>
</div>

View file

@ -104,6 +104,10 @@ test('Expect feedback form to to be GitHub issue feedback form', async () => {
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(title).toBeInTheDocument();
@ -116,6 +120,10 @@ test('Expect Preview on GitHub button to be disabled if there is no title or des
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
// default: disabled
@ -149,6 +157,10 @@ test.each([
category: category as GitHubFeedbackCategory,
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(title).toHaveProperty('placeholder', placeholders.title);
@ -158,17 +170,21 @@ test.each([
test.each([
{
category: 'bug',
link: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Fbug%20%F0%9F%90%9E%22',
link: '/bug/link',
},
{
category: 'feature',
link: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Ffeature%20%F0%9F%92%A1%22',
link: '/feature/link',
},
])('$category should have specific issues link', async ({ category, link }) => {
const { getByLabelText } = renderGitHubIssueFeedback({
category: category as GitHubFeedbackCategory,
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
const existingIssues = getByLabelText('GitHub issues');
@ -187,6 +203,10 @@ test.each<GitHubFeedbackCategory>([
category: category,
onCloseForm: onCloseFormMock,
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
// type dummy data
@ -216,6 +236,10 @@ describe('includeSystemInfo', () => {
category: 'feature',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(includeSystemInfo).toBeUndefined();
});
@ -225,6 +249,10 @@ describe('includeSystemInfo', () => {
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(includeSystemInfo).toBeInTheDocument();
@ -237,6 +265,10 @@ describe('includeSystemInfo', () => {
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
if (!includeSystemInfo) throw new Error('includeSystemInfo should be defined');
@ -265,6 +297,10 @@ describe('includeExtensionInfo', () => {
category: 'feature',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(includeExtensionInfo).toBeUndefined();
});
@ -274,6 +310,10 @@ describe('includeExtensionInfo', () => {
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(includeExtensionInfo).toBeInTheDocument();
@ -286,6 +326,10 @@ describe('includeExtensionInfo', () => {
category: 'bug',
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
if (!includeExtensionInfo) throw new Error('includeExtensionInfo should be defined');
@ -316,6 +360,10 @@ test.each<GitHubFeedbackCategory>([
category: category,
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(window.telemetryTrack).toHaveBeenNthCalledWith(1, `feedback.FormOpened`, { feedbackCategory: category });
@ -338,6 +386,10 @@ test.each<GitHubFeedbackCategory>([
category: category,
onCloseForm: vi.fn(),
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
expect(window.telemetryTrack).toHaveBeenNthCalledWith(1, `feedback.FormOpened`, { feedbackCategory: category });
@ -360,6 +412,10 @@ test('Expect close confirmation to be true if cancel clicked', async () => {
category: 'bug',
onCloseForm: closeMock,
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
// click on a cancel
@ -376,6 +432,10 @@ test('Expect opening existing GitHub issues to not close the feedback window', a
category: 'bug',
onCloseForm: onCloseFormMock,
contentChange: vi.fn(),
categoryLinks: {
bug: '/bug/link',
feature: '/feature/link',
},
});
const gitHubLink = getByLabelText('GitHub issues');

View file

@ -7,11 +7,12 @@ import FeedbackForm from '/@/lib/feedback/FeedbackForm.svelte';
interface Props {
onCloseForm: (confirm: boolean) => void;
categoryLinks: { [category: string]: string } | undefined;
category: GitHubFeedbackCategory;
contentChange: (e: boolean) => void;
}
let { onCloseForm, category = 'bug', contentChange }: Props = $props();
let { onCloseForm, categoryLinks = {}, category = 'bug', contentChange }: Props = $props();
let issueTitle = $state('');
let issueDescription = $state('');
@ -33,13 +34,9 @@ let issueValidationError = $derived.by(() => {
}
});
let titlePlaceholder = $state(category === 'bug' ? 'Bug Report Title' : 'Feature name');
let descriptionPlaceholder = $state(category === 'bug' ? 'Bug description' : 'Feature description');
let existingIssuesLink = $state(
category === 'bug'
? 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Fbug%20%F0%9F%90%9E%22'
: 'https://github.com/podman-desktop/podman-desktop/issues?q=label%3A%22kind%2Ffeature%20%F0%9F%92%A1%22',
);
let titlePlaceholder = $derived(category === 'bug' ? 'Bug Report Title' : 'Feature name');
let descriptionPlaceholder = $derived(category === 'bug' ? 'Bug description' : 'Feature description');
let existingIssuesLink = $derived(category === 'bug' ? categoryLinks.bug : categoryLinks.feature);
$effect(() => contentChange(Boolean(issueTitle || issueDescription)));
@ -48,7 +45,9 @@ onMount(async () => {
});
async function openGitHubIssues(): Promise<void> {
await window.openExternal(existingIssuesLink);
if (existingIssuesLink || categoryLinks.issues) {
await window.openExternal(existingIssuesLink ?? categoryLinks.issues);
}
}
async function previewOnGitHub(): Promise<void> {
@ -80,7 +79,7 @@ async function previewOnGitHub(): Promise<void> {
<FeedbackForm>
<svelte:fragment slot="content">
<div class="text-sm text-[var(--pd-state-warning)]">You can search existing {category} issues on <Link aria-label="GitHub issues" onclick={openGitHubIssues}>github.com/podman-desktop/podman-desktop/issues</Link></div>
<div class="text-sm text-[var(--pd-state-warning)]">You can search <Link aria-label="GitHub issues" onclick={openGitHubIssues}>existing {category} issues on GitHub</Link></div>
<!-- issue title -->
<label for="issueTitle" class="block mt-4 mb-2 text-sm font-medium text-[var(--pd-modal-text)]">Title</label>
<input

View file

@ -157,5 +157,11 @@
"placeholder": "Search images, containers, pods, and other resources"
}
]
}
},
"GitHubFeedbackLinks": {
"bug": "https://github.com/podman-desktop/podman-desktop/issues?q=is%3Aissue%20state%3Aopen%20type%3ABug",
"feature": "https://github.com/podman-desktop/podman-desktop/issues?q=is%3Aissue%20state%3Aopen%20type%3AFeature",
"issues": "https://github.com/podman-desktop/podman-desktop/issues"
},
"feedbackLinks": {}
}