Skip to content

Commit 8574760

Browse files
[Feature] side by side toolview (#588)
* implemented correct split functionality * added test cases plus update the PR template
1 parent 3822c59 commit 8574760

16 files changed

Lines changed: 2116 additions & 23 deletions

File tree

.github/pull_request_template.md

Lines changed: 75 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,76 @@
1-
Please fill in this template.
1+
<!-- PR title format: [Type] Brief description — e.g. [Feature] Add connection export -->
2+
<!-- Types: [Feature] | [Fix] | [Docs] | [Refactor] | [Chore] | [Test] -->
3+
<!-- Target branch: dev (not main) -->
24

3-
- [ ] Use a meaningful title for the pull request.
4-
- [ ] Follow the guidelines from the [CONTRIBUTING.md](https://github.com/PowerPlatformToolBox/desktop-app/CONTRIBUTING.md#pull-requests).
5-
- [ ] Mention the bug or the feature number the PR will be targeting.
6-
- [ ] Test the change in your own code. (Compile and run)
7-
- [ ] Resolve all GH Copilot comments.
5+
## Summary
6+
7+
<!-- One or two sentences describing what this PR does and why. -->
8+
9+
Closes #<!-- issue number -->
10+
11+
## Type of change
12+
13+
- [ ] New feature
14+
- [ ] Bug fix
15+
- [ ] Refactor (no functional change)
16+
- [ ] Documentation
17+
- [ ] Chore / maintenance (dependency update, build, config)
18+
- [ ] Test addition / improvement
19+
20+
## Changes
21+
22+
<!-- List the key files/areas changed and what was done. Be specific enough for a reviewer to navigate the diff. -->
23+
24+
-
25+
26+
## Architecture checklist
27+
28+
### Packages (`types` & `validation`)
29+
30+
<!-- Only fill in this section if you touched files under packages/. Otherwise delete it. -->
31+
32+
- [ ] **Not applicable** — no changes to `packages/`
33+
34+
If you did change a package:
35+
36+
- [ ] `@pptb/types` (`types`): type definitions updated and version bumped in `packages/types/package.json`
37+
- [ ] `@pptb/validate` (`validation`): validation rules updated and version bumped in `packages/validation/package.json`
38+
39+
### Code quality
40+
41+
- [ ] `pnpm run typecheck` passes with **0 errors** (warnings are acceptable)
42+
- [ ] `pnpm run lint` passes with **0 errors** (warnings are acceptable)
43+
- [ ] `pnpm run build` completes successfully
44+
45+
## Testing
46+
47+
<!-- Describe how you verified this change works. -->
48+
49+
- [ ] `pnpm run test:unit` passes (for changes to `src/main/`, `src/common/`, or `src/renderer/` utilities)
50+
- [ ] `pnpm run test:e2e` passes (for UI / navigation / end-to-end flows)
51+
- [ ] Manually tested in the running app (`pnpm run dev`)
52+
53+
**Scenario tested:**
54+
55+
<!-- e.g. "Opened the app, navigated to X, confirmed Y behaviour" -->
56+
57+
## Screenshots / recordings
58+
59+
<!-- Attach screenshots or a short screen recording for any UI change. Delete this section if not applicable. -->
60+
61+
## Breaking changes
62+
63+
<!-- Does this change affect the tool API (`toolboxAPI` / `dataverseAPI`), existing IPC channels, or `pptoolbox-types`? -->
64+
65+
- [ ] No breaking changes
66+
- [ ] Yes — describe impact and migration path below:
67+
68+
<!-- Breaking change details -->
69+
70+
## Reviewer notes
71+
72+
<!-- Anything specific you'd like reviewers to focus on, or context that helps them review faster. -->
73+
74+
- [ ] I have added appropriate unit and/or e2e tests for this change
75+
- [ ] I have resolved all GitHub Copilot review comments
76+
- [ ] I have followed the guidelines in [CONTRIBUTING.md](https://github.com/PowerPlatformToolBox/desktop-app/blob/dev/CONTRIBUTING.md#pull-requests)

.github/workflows/pr-checklist.yml

Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
name: PR Checklist Validation
2+
3+
on:
4+
pull_request:
5+
types: [opened, edited, synchronize, reopened]
6+
branches:
7+
- main
8+
- dev
9+
10+
permissions:
11+
pull-requests: write
12+
issues: write
13+
contents: read
14+
15+
jobs:
16+
validate-checklist:
17+
name: PR Checklist
18+
runs-on: ubuntu-latest
19+
20+
steps:
21+
# -----------------------------------------------------------------
22+
# Parse the PR body, validate required checkboxes, manage labels,
23+
# and post/update a single sticky comment with the result.
24+
# -----------------------------------------------------------------
25+
- name: Validate checklist and apply labels
26+
uses: actions/github-script@v7
27+
with:
28+
script: |
29+
const body = context.payload.pull_request.body || '';
30+
const title = context.payload.pull_request.title || '';
31+
const prNumber = context.payload.pull_request.number;
32+
const author = context.payload.pull_request.user.login;
33+
const existingLabels = context.payload.pull_request.labels.map(l => l.name);
34+
35+
// ── Helpers ─────────────────────────────────────────────────────────
36+
37+
/**
38+
* Extract the content of a Markdown section identified by its heading
39+
* prefix (e.g. "## Testing" or "### Code quality"). Stops as soon as
40+
* it encounters another heading at the same level or higher.
41+
*/
42+
function extractSection(text, headingPrefix) {
43+
const lines = text.split('\n');
44+
const headingLevel = (headingPrefix.match(/^(#+)/) || ['', '#'])[1].length;
45+
let capturing = false;
46+
const result = [];
47+
48+
for (const line of lines) {
49+
if (!capturing && line.startsWith(headingPrefix)) {
50+
capturing = true;
51+
result.push(line);
52+
} else if (capturing) {
53+
const m = line.match(/^(#+)\s/);
54+
if (m && m[1].length <= headingLevel) break;
55+
result.push(line);
56+
}
57+
}
58+
return result.join('\n');
59+
}
60+
61+
/** True if the section contains at least one `- [x]` line. */
62+
function atLeastOneChecked(section) {
63+
return /^- \[x\]/im.test(section);
64+
}
65+
66+
/** True if every checkbox in the section is `- [x]` (none are `- [ ]`). */
67+
function allChecked(section) {
68+
return atLeastOneChecked(section) && !/^- \[ \]/im.test(section);
69+
}
70+
71+
// ── Extract relevant sections ────────────────────────────────────────
72+
73+
const typeSection = extractSection(body, '## Type of change');
74+
const packagesSection = extractSection(body, '### Packages');
75+
const codeQuality = extractSection(body, '### Code quality');
76+
const testingSection = extractSection(body, '## Testing');
77+
const breakingSection = extractSection(body, '## Breaking changes');
78+
const reviewerSection = extractSection(body, '## Reviewer notes');
79+
80+
// ── Validation rules ─────────────────────────────────────────────────
81+
82+
const errors = [];
83+
84+
// PR title must follow [Type] Brief description
85+
if (!/^\[(Feature|Fix|Docs|Refactor|Chore|Test)\] .+/.test(title)) {
86+
errors.push(
87+
'**PR title** must follow `[Type] Brief description` ' +
88+
'— valid types: `Feature` · `Fix` · `Docs` · `Refactor` · `Chore` · `Test`'
89+
);
90+
}
91+
92+
// Type of change — at least one
93+
if (!atLeastOneChecked(typeSection)) {
94+
errors.push('**Type of change** — select at least one option');
95+
}
96+
97+
// Packages — at least one (including "Not applicable")
98+
if (!atLeastOneChecked(packagesSection)) {
99+
errors.push(
100+
'**Packages** — select at least one option ' +
101+
'(tick "Not applicable" if no package changes were made)'
102+
);
103+
}
104+
105+
// Code quality — all boxes must be ticked
106+
if (!allChecked(codeQuality)) {
107+
errors.push('**Code quality** — all checkboxes must be ticked before merging');
108+
}
109+
110+
// Testing — all boxes must be ticked
111+
if (!allChecked(testingSection)) {
112+
errors.push('**Testing** — all checkboxes must be ticked before merging');
113+
}
114+
115+
// Breaking changes — at least one option selected
116+
if (!atLeastOneChecked(breakingSection)) {
117+
errors.push('**Breaking changes** — select at least one option');
118+
}
119+
120+
// Reviewer notes — all boxes must be ticked
121+
if (!allChecked(reviewerSection)) {
122+
errors.push('**Reviewer notes** — all checkboxes must be ticked before merging');
123+
}
124+
125+
// ── Label management ─────────────────────────────────────────────────
126+
127+
// breaking-change: the "Yes — describe impact" line is checked
128+
const isBreakingChange = /^- \[x\].*Yes/im.test(breakingSection);
129+
130+
// package-changed: any @pptb line is checked (i.e. not just "Not applicable")
131+
const isPackageChanged = /^- \[x\].*@pptb/im.test(packagesSection);
132+
133+
async function syncLabel(name, shouldHave) {
134+
const has = existingLabels.includes(name);
135+
if (shouldHave && !has) {
136+
await github.rest.issues.addLabels({
137+
owner: context.repo.owner,
138+
repo: context.repo.repo,
139+
issue_number: prNumber,
140+
labels: [name],
141+
});
142+
} else if (!shouldHave && has) {
143+
try {
144+
await github.rest.issues.removeLabel({
145+
owner: context.repo.owner,
146+
repo: context.repo.repo,
147+
issue_number: prNumber,
148+
name,
149+
});
150+
} catch {
151+
// Label may have already been removed — ignore
152+
}
153+
}
154+
}
155+
156+
await syncLabel('breaking-change', isBreakingChange);
157+
await syncLabel('package-changed', isPackageChanged);
158+
159+
// ── Sticky comment ───────────────────────────────────────────────────
160+
// We post one comment and update it in place on subsequent runs so the
161+
// PR timeline stays clean.
162+
163+
const MARKER = '<!-- pr-checklist-bot -->';
164+
165+
const { data: comments } = await github.rest.issues.listComments({
166+
owner: context.repo.owner,
167+
repo: context.repo.repo,
168+
issue_number: prNumber,
169+
});
170+
const existing = comments.find(c => c.body && c.body.includes(MARKER));
171+
172+
const labelNote = [
173+
isBreakingChange ? '🔴 `breaking-change` label applied' : null,
174+
isPackageChanged ? '🔵 `package-changed` label applied' : null,
175+
].filter(Boolean).join('\n');
176+
177+
let commentBody;
178+
179+
const errorList = errors.map(e => `- ${e}`).join('\n');
180+
181+
if (errors.length === 0) {
182+
commentBody =
183+
`${MARKER}\n` +
184+
`### ✅ PR Checklist\n\n` +
185+
`All required checklist items are complete. This PR is ready for review.\n` +
186+
(labelNote ? `\n${labelNote}` : '');
187+
} else {
188+
// On updates we omit the @mention to avoid re-notifying on every push.
189+
const mention = existing ? '' : `@${author} — please address the items below.\n\n`;
190+
commentBody =
191+
`${MARKER}\n` +
192+
`### ❌ PR Checklist — Action Required\n\n` +
193+
`${mention}` +
194+
`The following items must be completed before this PR can be merged:\n\n` +
195+
`${errorList}\n` +
196+
(labelNote ? `\n---\n${labelNote}` : '');
197+
}
198+
199+
if (existing) {
200+
await github.rest.issues.updateComment({
201+
owner: context.repo.owner,
202+
repo: context.repo.repo,
203+
comment_id: existing.id,
204+
body: commentBody,
205+
});
206+
} else {
207+
await github.rest.issues.createComment({
208+
owner: context.repo.owner,
209+
repo: context.repo.repo,
210+
issue_number: prNumber,
211+
body: commentBody,
212+
});
213+
}
214+
215+
// ── Fail the status check ────────────────────────────────────────────
216+
217+
if (errors.length > 0) {
218+
core.setFailed(
219+
`PR checklist incomplete — ${errors.length} item${errors.length > 1 ? 's' : ''} outstanding:\n` +
220+
errors.map(e => ` • ${e.replace(/\*\*/g, '')}`).join('\n')
221+
);
222+
}

src/common/ipc/channels.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,5 +285,21 @@ export const MODAL_WINDOW_CHANNELS = {
285285
RENDERER_MESSAGE: "modal-window:renderer-message",
286286
} as const;
287287

288+
// Split layout channels
289+
export const SPLIT_LAYOUT_CHANNELS = {
290+
ACTIVATE: "split-layout:activate",
291+
DEACTIVATE: "split-layout:deactivate",
292+
SET_RATIO: "split-layout:set-ratio",
293+
GET_STATE: "split-layout:get-state",
294+
/** setActiveInPane(pane, instanceId) — make a group member the visible tool in its pane */
295+
SWITCH_PANE: "split-layout:switch-pane",
296+
/** moveToPane(instanceId, targetPane) — move a tab from one group to another */
297+
MOVE_TO_PANE: "split-layout:move-to-pane",
298+
/** setFocusedPane(pane) — set which pane receives newly opened tools */
299+
FOCUS_PANE: "split-layout:focus-pane",
300+
/** Pushed from main to renderer when split state changes */
301+
STATE_CHANGED: "split-layout:state-changed",
302+
} as const;
303+
288304
// Type helper to extract channel names
289305
export type ChannelName<T> = T[keyof T];

src/common/types/api.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,41 @@ export interface DataverseAPI {
159159
getEntitySetName: (entityLogicalName: string) => Promise<string>;
160160
}
161161

162+
/**
163+
* Split layout state returned by the main process
164+
*/
165+
export interface SplitLayoutState {
166+
isActive: boolean;
167+
/** Ordered list of instanceIds assigned to the left pane. */
168+
leftGroup: string[];
169+
/** Ordered list of instanceIds assigned to the right pane. */
170+
rightGroup: string[];
171+
/** The currently visible (active) tool in the left pane. */
172+
activeLeftInstanceId: string | null;
173+
/** The currently visible (active) tool in the right pane. */
174+
activeRightInstanceId: string | null;
175+
/** Which pane receives newly opened tools. */
176+
focusedPane: "left" | "right";
177+
ratio: number;
178+
}
179+
180+
/**
181+
* Split Layout API namespace
182+
*/
183+
export interface SplitLayoutAPI {
184+
activate: (leftInstanceId: string, rightInstanceId: string) => Promise<boolean>;
185+
deactivate: () => Promise<boolean>;
186+
setRatio: (ratio: number) => Promise<void>;
187+
getState: () => Promise<SplitLayoutState>;
188+
/** Make instanceId the active (visible) tool in its pane. Also focuses that pane. */
189+
switchPane: (pane: "left" | "right", instanceId: string) => Promise<boolean>;
190+
/** Move instanceId from its current group to targetPane. Collapses split if source becomes empty. */
191+
moveToPane: (instanceId: string, targetPane: "left" | "right") => Promise<boolean>;
192+
/** Set which pane receives newly opened tools. */
193+
focusPane: (pane: "left" | "right") => Promise<void>;
194+
onStateChanged: (callback: (state: SplitLayoutState) => void) => void;
195+
}
196+
162197
/**
163198
* Main ToolboxAPI interface
164199
*/
@@ -276,6 +311,9 @@ export interface ToolboxAPI {
276311
/** Install the beta (pre-release) npm package for a registry tool and return the loaded Tool. */
277312
installPrereleaseToolFromNpm: (npmPackageName: string) => Promise<Tool>;
278313

314+
// Split layout namespace
315+
splitLayout: SplitLayoutAPI;
316+
279317
// Utils namespace
280318
utils: UtilsAPI;
281319

src/common/types/settings.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,5 @@ export interface UserSettings {
9595
categoryColorThickness?: number; // Thickness in pixels of the category color border under the tab
9696
environmentColorThickness?: number; // Thickness in pixels of the environment color border around the tool panel
9797
mcpAccessToken?: string; // Access token for local MCP server authentication
98+
splitDividerRatio?: number; // Persisted position of the split-pane divider (0.15–0.85)
9899
}

0 commit comments

Comments
 (0)