Skip to content

Commit

Permalink
fix: improve default contribution action icon
Browse files Browse the repository at this point in the history
We've had several people get confused with actions contributed by
extensions that show up as an ellipsis icon in popup menus and Detail
pages. The correct/full fix will require us to allow commands to provide
their own icons, and to do this for all of the current extensions. This
is not that fix. :)

The other two things we should do is switch the default icon so that it
doesn't use the same icon as our popup menus, and to pass the detailed
property through the ContributionActions component so that these icons
show up on buttons on the Details pages like the other actions. This is
that part of the fix!

I went through FA trying to find a more suitable icon and this is the
best I could come up with. I preferred it to icons like the puzzle
piece and I think it still gives some indication that the action comes
from an extension.

First step toward fixing #4763.

Signed-off-by: Tim deBoer <git@tdeboer.ca>
  • Loading branch information
deboer-tim authored and benoitf committed Dec 12, 2023
1 parent ee164a3 commit 8b9619a
Show file tree
Hide file tree
Showing 6 changed files with 43 additions and 2 deletions.
35 changes: 35 additions & 0 deletions packages/renderer/src/lib/actions/ContributionActions.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,41 @@ test('Expect one ListItemButtonIcon', async () => {
expect(item).toBeInTheDocument();
});

test('Expect one ListItemButtonIcon without detail', async () => {
render(ContributionActions, {
args: [],
contributions: [
{
command: 'dummy.command',
title: 'dummy-title',
},
],
onError: () => {},
dropdownMenu: false,
});
const item = screen.getByLabelText('dummy-title');
expect(item).toBeInTheDocument();
expect(item).toHaveClass('m-0.5');
});

test('Expect one ListItemButtonIcon with detail', async () => {
render(ContributionActions, {
args: [],
contributions: [
{
command: 'dummy.command',
title: 'dummy-title',
},
],
onError: () => {},
dropdownMenu: false,
detailed: true,
});
const item = screen.getByLabelText('dummy-title');
expect(item).toBeInTheDocument();
expect(item).not.toHaveClass('m-0.5');
});

test('Expect executeCommand to be called', async () => {
render(ContributionActions, {
args: [],
Expand Down
6 changes: 4 additions & 2 deletions packages/renderer/src/lib/actions/ContributionActions.svelte
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<script lang="ts">
import type { Menu } from '../../../../main/src/plugin/menu-registry';
import { faEllipsisVertical } from '@fortawesome/free-solid-svg-icons';
import { faPlug } from '@fortawesome/free-solid-svg-icons';
import ListItemButtonIcon from '../ui/ListItemButtonIcon.svelte';
import { removeNonSerializableProperties } from '/@/lib/actions/ActionUtils';
import type { ContextUI } from '/@/lib/context/context';
Expand All @@ -16,6 +16,7 @@ export let contextPrefix: string | undefined = undefined;
export let dropdownMenu = false;
export let contributions: Menu[] = [];
export let detailed = false;
let filteredContributions: Menu[] = [];
$: {
Expand Down Expand Up @@ -75,6 +76,7 @@ async function executeContribution(menu: Menu): Promise<void> {
title="{menu.title}"
onClick="{() => executeContribution(menu)}"
menu="{dropdownMenu}"
icon="{faEllipsisVertical}"
icon="{faPlug}"
detailed="{detailed}"
disabledWhen="{menu.disabled}" />
{/each}
1 change: 1 addition & 0 deletions packages/renderer/src/lib/compose/ComposeActions.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -138,5 +138,6 @@ if (dropdownMenu) {
contextPrefix="composeItem"
dropdownMenu="{dropdownMenu}"
contributions="{contributions}"
detailed="{detailed}"
onError="{errorCallback}" />
</svelte:component>
Original file line number Diff line number Diff line change
Expand Up @@ -187,5 +187,6 @@ if (dropdownMenu) {
contextPrefix="containerItem"
dropdownMenu="{dropdownMenu}"
contributions="{contributions}"
detailed="{detailed}"
onError="{errorCallback}" />
</svelte:component>
1 change: 1 addition & 0 deletions packages/renderer/src/lib/image/ImageActions.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ function onError(error: string): void {
dropdownMenu="{dropdownMenu}"
contributions="{contributions}"
contextPrefix="imageItem"
detailed="{detailed}"
onError="{onError}" />

{#if errorMessage}
Expand Down
1 change: 1 addition & 0 deletions packages/renderer/src/lib/pod/PodActions.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -202,5 +202,6 @@ if (dropdownMenu) {
contextPrefix="podItem"
dropdownMenu="{dropdownMenu}"
contributions="{contributions}"
detailed="{detailed}"
onError="{errorCallback}" />
</svelte:component>

0 comments on commit 8b9619a

Please sign in to comment.