Skip to content

Commit 87196a8

Browse files
mtsgrdmtsgrd
committed
Convert more services to singletons
Because of how setContext/getContext works we only want global singleton services, with `projectId` typically passed to service methods.
1 parent 2694944 commit 87196a8

File tree

13 files changed

+120
-168
lines changed

13 files changed

+120
-168
lines changed

‎apps/desktop/cypress/e2e/review.cy.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ describe('Review', () => {
1616
mockCommand('changes_in_worktree', (params) => mockBackend.getWorktreeChanges(params));
1717
mockCommand('tree_change_diffs', (params) => mockBackend.getDiff(params));
1818
mockCommand('get_base_branch_data', () => mockBackend.getBaseBranchData());
19-
mockCommand('get_available_review_templates', () => mockBackend.getAvailableReviewTemplates());
19+
mockCommand('pr_templates', () => mockBackend.getAvailableReviewTemplates());
2020
mockCommand('push_stack', (params) => mockBackend.pushStack(params));
2121
mockCommand('list_remotes', (params) => mockBackend.listRemotes(params));
2222
mockCommand('update_branch_pr_number', (params) => mockBackend.updateBranchPrNumber(params));
2323
mockCommand('hunk_assignments', (params) => mockBackend.getHunkAssignments(params));
24-
mockCommand('get_review_template_contents', (args) => mockBackend.getTemplateContent(args));
24+
mockCommand('pr_template', (args) => mockBackend.getTemplateContent(args));
2525

2626
cy.intercept(
2727
{
@@ -406,12 +406,12 @@ describe('Review - stacked branches', () => {
406406
mockCommand('changes_in_worktree', (params) => mockBackend.getWorktreeChanges(params));
407407
mockCommand('tree_change_diffs', (params) => mockBackend.getDiff(params));
408408
mockCommand('get_base_branch_data', () => mockBackend.getBaseBranchData());
409-
mockCommand('get_available_review_templates', () => mockBackend.getAvailableReviewTemplates());
409+
mockCommand('pr_templates', () => mockBackend.getAvailableReviewTemplates());
410410
mockCommand('push_stack', (params) => mockBackend.pushStack(params));
411411
mockCommand('list_remotes', (params) => mockBackend.listRemotes(params));
412412
mockCommand('update_branch_pr_number', (params) => mockBackend.updateBranchPrNumber(params));
413413
mockCommand('hunk_assignments', (params) => mockBackend.getHunkAssignments(params));
414-
mockCommand('get_review_template_contents', (args) => mockBackend.getTemplateContent(args));
414+
mockCommand('pr_template', (args) => mockBackend.getTemplateContent(args));
415415

416416
cy.intercept(
417417
{

‎apps/desktop/src/components/CommitDetails.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@
3535
// Calculate approximately how many characters fit on one line, as a
3636
// function of container width as well as zoom level.
3737
// TODO: Turn this magic formula into something meaningful.
38-
const fontFactor = $rewrapCommitMessage ? 2.3 : 2;
38+
const fontFactor = $derived($rewrapCommitMessage ? 2.3 : 1.99);
3939
const maxLength = $derived((messageWidthRem - 2) * fontFactor - (Math.pow(zoom, 2) - 1));
4040
4141
const message = $derived(commit.message);

‎apps/desktop/src/components/Drawer.svelte

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -73,21 +73,6 @@
7373
let headerHeight = $state(0);
7474
let contentHeight = $state(0);
7575
const totalHeightRem = $derived(pxToRem(headerHeight + 1 + contentHeight, zoom));
76-
77-
let resizerInstance = $state<Resizer>();
78-
$effect(() => {
79-
// Reset resizer if we happen on a value that equals the scroll
80-
// height, enabling the user to more easily undo manual sizing. It
81-
// is assumed that an unset value makes the element display in
82-
// full, otherwise there would be sudden content shift.
83-
// TODO: Figure out why we need to +1 the total height.
84-
const totalHeight = headerHeight + contentHeight + 1;
85-
if (clientHeight === totalHeight) {
86-
requestAnimationFrame(() => {
87-
resizerInstance?.setValue(undefined);
88-
});
89-
}
90-
});
9176
</script>
9277

9378
<div
@@ -169,18 +154,24 @@
169154
</ConfigurableScrollableContainer>
170155
{/if}
171156
{#if resizer}
157+
<!--
158+
This ternarny statement captures the nuance of maxHeight possibly
159+
being lower than minHeight.
160+
TODO: Move this logic into the resizer so it applies everwhere.
161+
-->
162+
{@const maxHeight =
163+
resizer.maxHeight && resizer.minHeight
164+
? Math.min(resizer.maxHeight, Math.max(totalHeightRem, resizer.minHeight))
165+
: undefined}
172166
<Resizer
173-
bind:this={resizerInstance}
174-
defaultValue={undefined}
175167
viewport={containerDiv}
168+
defaultValue={undefined}
169+
passive={resizer.passive}
176170
hidden={$collapsed}
177171
direction="down"
178172
imitateBorder
179173
{...resizer}
180-
maxHeight={resizer.maxHeight && resizer.minHeight
181-
? Math.min(resizer.maxHeight, Math.max(totalHeightRem, resizer.minHeight))
182-
: undefined}
183-
passive={resizer.passive}
174+
{maxHeight}
184175
/>
185176
{/if}
186177
</div>

‎apps/desktop/src/components/PrTemplateSection.svelte

Lines changed: 62 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,90 @@
11
<script lang="ts">
2+
import ReduxResult from '$components/ReduxResult.svelte';
3+
import { StackService } from '$lib/stacks/stackService.svelte';
24
import { TestId } from '$lib/testing/testIds';
5+
import { inject } from '@gitbutler/shared/context';
36
import Toggle from '@gitbutler/ui/Toggle.svelte';
47
import Select from '@gitbutler/ui/select/Select.svelte';
58
import SelectItem from '@gitbutler/ui/select/SelectItem.svelte';
6-
import type { PrTemplateStore } from '$lib/forge/prContents';
9+
import type { Writable } from 'svelte/store';
710
811
interface Props {
912
projectId: string;
13+
forgeName: string;
1014
disabled: boolean;
15+
template: {
16+
enabled: Writable<boolean>;
17+
path: Writable<string | undefined>;
18+
};
1119
onselect: (template: string) => void;
12-
templateStore: PrTemplateStore;
1320
}
1421
15-
let { templateStore, disabled, onselect }: Props = $props();
22+
const { projectId, forgeName, template, disabled, onselect }: Props = $props();
1623
17-
const templatePath = $derived(templateStore.templatePath);
18-
const templateEnabled = $derived(templateStore.templateEnabled);
24+
const [stackService] = inject(StackService);
25+
26+
const path = $derived(template.path);
27+
const enabled = $derived(template.enabled);
1928
2029
// Available pull request templates.
21-
const templatesResult = $derived(templateStore.getAvailable());
30+
const templatesResult = $derived(stackService.templates(projectId, forgeName));
2231
23-
async function selectTemplate(path: string) {
24-
const template = await templateStore.getTemplateContent(path);
25-
templatePath.set(path);
26-
onselect(template);
32+
async function selectTemplate(newPath: string) {
33+
const template = await stackService.template(projectId, forgeName, newPath);
34+
if (template.data) {
35+
path.set(newPath);
36+
onselect(template.data);
37+
}
2738
}
2839
29-
async function setEnabled(enabled: boolean) {
30-
const ts = await templatesResult;
31-
templateEnabled.set(enabled);
32-
if (enabled) {
33-
const path = $templatePath ? $templatePath : ts.at(0);
40+
async function setEnabled(value: boolean) {
41+
const ts = templatesResult;
42+
enabled.set(value);
43+
if (value) {
44+
const path = $path ? $path : ts.current?.data?.at(0);
3445
if (path) {
3546
selectTemplate(path);
3647
}
3748
}
3849
}
3950
</script>
4051

41-
{#await templatesResult then templates}
42-
{#if templates.length > 0}
43-
<div class="pr-template__wrap">
44-
<label class="pr-template__toggle" for="pr-template-toggle">
45-
<span class="text-13 text-semibold">Use template</span>
46-
<Toggle
47-
testId={TestId.ReviewTemplateToggle}
48-
small
49-
id="pr-template-toggle"
50-
onchange={(checked) => setEnabled(checked)}
51-
checked={$templateEnabled}
52-
disabled={templates.length === 0 || disabled}
53-
/>
54-
</label>
55-
<Select
56-
value={$templatePath}
57-
options={templates.map((value) => ({ label: value, value }))}
58-
placeholder={templates.length > 0 ? 'Choose template' : 'No PR templates found ¯\\_(ツ)_/¯'}
59-
flex="1"
60-
searchable
61-
disabled={!$templateEnabled || templates.length === 0 || disabled}
62-
onselect={(path) => selectTemplate(path)}
63-
>
64-
{#snippet itemSnippet({ item, highlighted })}
65-
<SelectItem selected={item.value === $templatePath} {highlighted}>
66-
{item.label}
67-
</SelectItem>
68-
{/snippet}
69-
</Select>
70-
</div>
71-
{/if}
72-
{/await}
52+
<ReduxResult {projectId} result={templatesResult.current}>
53+
{#snippet children(templates)}
54+
{#if templates && templates.length > 0}
55+
<div class="pr-template__wrap">
56+
<label class="pr-template__toggle" for="pr-template-toggle">
57+
<span class="text-13 text-semibold">Use template</span>
58+
<Toggle
59+
testId={TestId.ReviewTemplateToggle}
60+
small
61+
id="pr-template-toggle"
62+
onchange={(checked) => setEnabled(checked)}
63+
checked={$enabled}
64+
disabled={templates.length === 0 || disabled}
65+
/>
66+
</label>
67+
<Select
68+
value={$path}
69+
options={templates.map((value) => ({ label: value, value }))}
70+
placeholder={templates.length > 0
71+
? 'Choose template'
72+
: 'No PR templates found ¯\\_(ツ)_/¯'}
73+
flex="1"
74+
searchable
75+
disabled={!$enabled || templates.length === 0 || disabled}
76+
onselect={(path) => selectTemplate(path)}
77+
>
78+
{#snippet itemSnippet({ item, highlighted })}
79+
<SelectItem selected={item.value === $path} {highlighted}>
80+
{item.label}
81+
</SelectItem>
82+
{/snippet}
83+
</Select>
84+
</div>
85+
{/if}
86+
{/snippet}
87+
</ReduxResult>
7388

7489
<style lang="postcss">
7590
.pr-template__wrap {

‎apps/desktop/src/components/ReviewCreation.svelte

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,8 @@
2222
import { mapErrorToToast } from '$lib/forge/github/errorMap';
2323
import { GitHubPrService } from '$lib/forge/github/githubPrService.svelte';
2424
import { type PullRequest } from '$lib/forge/interface/types';
25-
import { PrPersistedStore, PrTemplateStore } from '$lib/forge/prContents';
25+
import { PrPersistedStore } from '$lib/forge/prContents';
2626
import { updatePrDescriptionTables as updatePrStackInfo } from '$lib/forge/shared/prFooter';
27-
import { TemplateService } from '$lib/forge/templateService';
2827
import { showError, showToast } from '$lib/notifications/toasts';
2928
import { RemotesService } from '$lib/remotes/remotesService';
3029
import { requiresPush } from '$lib/stacks/stack';
@@ -61,7 +60,6 @@
6160
const aiService = getContext(AIService);
6261
const remotesService = getContext(RemotesService);
6362
const uiState = getContext(UiState);
64-
const templateService = getContext(TemplateService);
6563
6664
const user = userService.user;
6765
@@ -124,15 +122,15 @@
124122
return branchName;
125123
}
126124
127-
const templateStore = $derived(
128-
new PrTemplateStore(projectId, forge.current.name, templateService)
129-
);
130-
const templateEnabled = $derived(templateStore.templateEnabled);
131-
const templatePath = $derived(templateStore.templatePath);
125+
const templatePath = persisted<string | undefined>(undefined, `last-template-${projectId}`);
126+
const templateEnabled = persisted(false, `enable-template-${projectId}`);
132127
133128
async function getDefaultBody(commits: Commit[]): Promise<string> {
134129
if ($templateEnabled && $templatePath) {
135-
return await templateStore.getTemplateContent($templatePath);
130+
const result = await stackService.template(projectId, forge.current.name, $templatePath);
131+
if (result.data) {
132+
return result.data;
133+
}
136134
}
137135
if (commits.length === 1) {
138136
return splitMessage(commits[0]!.message).description;
@@ -365,7 +363,8 @@
365363
<AsyncRender>
366364
<PrTemplateSection
367365
{projectId}
368-
{templateStore}
366+
template={{ enabled: templateEnabled, path: templatePath }}
367+
forgeName={forge.current.name}
369368
disabled={isExecuting}
370369
onselect={(value) => {
371370
prBody.set(value);

‎apps/desktop/src/lib/branches/gitBranch.ts

Lines changed: 0 additions & 14 deletions
This file was deleted.

‎apps/desktop/src/lib/forge/prContents.ts

Lines changed: 1 addition & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
1-
import {
2-
getEphemeralStorageItem,
3-
persisted,
4-
setEphemeralStorageItem,
5-
type Persisted
6-
} from '@gitbutler/shared/persisted';
1+
import { getEphemeralStorageItem, setEphemeralStorageItem } from '@gitbutler/shared/persisted';
72
import { type Subscriber, type Unsubscriber } from 'svelte/store';
83
import type { Commit } from '$lib/branches/v3';
9-
import type { TemplateService } from '$lib/forge/templateService';
104

115
/**
126
* A custom persisted store that makes it easier to manage pr descriptions.
@@ -79,25 +73,3 @@ export class PrPersistedStore {
7973
function isEmptyOrUndefined(line?: string) {
8074
return line === '\n' || line === '' || line === undefined;
8175
}
82-
83-
export class PrTemplateStore {
84-
readonly templatePath: Persisted<string | undefined>;
85-
readonly templateEnabled: Persisted<boolean>;
86-
87-
constructor(
88-
private projectId: string,
89-
private forgeName: string,
90-
private templateService: TemplateService
91-
) {
92-
this.templatePath = persisted<string | undefined>(undefined, `last-template-${projectId}`);
93-
this.templateEnabled = persisted(false, `enable-template-${projectId}`);
94-
}
95-
96-
async getAvailable(): Promise<string[]> {
97-
return await this.templateService.getAvailable(this.forgeName);
98-
}
99-
100-
async getTemplateContent(templatePath: string): Promise<string> {
101-
return await this.templateService.getContent(this.forgeName, templatePath);
102-
}
103-
}

‎apps/desktop/src/lib/forge/templateService.ts

Lines changed: 0 additions & 20 deletions
This file was deleted.

‎apps/desktop/src/lib/stacks/stackService.svelte.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -821,6 +821,18 @@ export class StackService {
821821
])
822822
);
823823
}
824+
825+
templates(projectId: string, forgeName: string) {
826+
return this.api.endpoints.templates.useQuery({ projectId, forge: { name: forgeName } });
827+
}
828+
829+
async template(projectId: string, forgeName: string, relativePath: string) {
830+
return await this.api.endpoints.template.fetch({
831+
relativePath,
832+
projectId,
833+
forge: { name: forgeName }
834+
});
835+
}
824836
}
825837

826838
function injectEndpoints(api: ClientState['backendApi']) {
@@ -1568,6 +1580,17 @@ function injectEndpoints(api: ClientState['backendApi']) {
15681580
await lifecycleApi.cacheEntryRemoved;
15691581
unsubscribe();
15701582
}
1583+
}),
1584+
templates: build.query<string[], { projectId: string; forge: { name: string } }>({
1585+
extraOptions: { command: 'pr_templates' },
1586+
query: (args) => args
1587+
}),
1588+
template: build.query<
1589+
string,
1590+
{ projectId: string; forge: { name: string }; relativePath: string }
1591+
>({
1592+
extraOptions: { command: 'pr_template' },
1593+
query: (args) => args
15711594
})
15721595
})
15731596
});

0 commit comments

Comments
 (0)