Skip to content

Add sheet query parameter to preselect schematic sheet #153

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

Open
CapnKernel wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add sheet query parameter to preselect schematic sheet #153

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ccf4283
Add sheet query parameter to preselect schematic sheet
Dec 12, 2025
685a3db
Fixed some PR review comments: Removed a log, reformatted code
Dec 14, 2025
3a72dd4
If the sheet query arg was a number, it could match a UUID. Fixed.
Dec 14, 2025
503f52b
Simplified how sheet names are matched against "sheet" query arg
Dec 15, 2025
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
18 changes: 16 additions & 2 deletions src/kicanvas/elements/common/project-panel.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export class KCProjectPanelElement extends KCUIElement {
];

#menu: KCUIMenuElement;
#selected_name: string | null = null;

@XiangYyang XiangYyang Dec 12, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not totally sure what the purpose of this attribute is. I tried removing it, and it seems to work just fine. I'd really appreciate it if you could explain it to me.

@CapnKernel CapnKernel Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Did you see my response to this? I wrote one, but perhaps I forgot to save it.

If you click on a schematic name in the list on the right, it is visually highlighted. That should also happen if we select a schematic with sheet=. This code is necessary to highlight the sheet name if we use sheet=.

@XiangYyang XiangYyang Dec 16, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Alright, I haven't actually received that response yet. Anyway, this appears to be a component lifecycle issue. Perhaps we should think about opening another PR to fix it.

@XiangYyang XiangYyang Dec 16, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think there are indeed some imperfections here. Maybe something about initialContentCallback and it loaded but no event handler was registered. I would really recommend that we address the root cause of this issue. Well, anyway, it seems like this should be handled in another PR. After this issue is fixed, changes to file project-panel.ts will no longer be necessary. So we just accept its imperfections.

project: Project;

override connectedCallback() {
Expand Down Expand Up @@ -105,11 +106,14 @@ export class KCProjectPanelElement extends KCUIElement {
}

get selected() {
return this.#menu.selected?.name ?? null;
return this.#menu?.selected?.name ?? this.#selected_name;
}

set selected(name: string | null) {
this.#menu.selected = name;
this.#selected_name = name;
if (this.#menu) {
this.#menu.selected = name;
}
}

@no_self_recursion
Expand All @@ -124,6 +128,12 @@ export class KCProjectPanelElement extends KCUIElement {
return html``;
}

// Update selected_name from active page if not already set
const active_path = this.project.active_page?.project_path ?? null;
if (active_path !== null && this.#selected_name !== active_path) {
this.#selected_name = active_path;
}

for (const page of this.project.pages()) {
const icon =
page.type == "schematic"
Expand Down Expand Up @@ -161,6 +171,10 @@ export class KCProjectPanelElement extends KCUIElement {
${file_btn_elms}
</kc-ui-menu>` as KCUIMenuElement;

if (this.#selected_name !== null) {
this.#menu.selected = this.#selected_name;
}

return html`<kc-ui-panel>
<kc-ui-panel-title title="Project"></kc-ui-panel-title>
<kc-ui-panel-body>${this.#menu}</kc-ui-panel-body>
Expand Down
27 changes: 26 additions & 1 deletion src/kicanvas/elements/kicanvas-shell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,32 @@ class KiCanvasShellElement extends KCUIElement {

try {
await this.project.load(vfs);
this.project.set_active_page(this.project.first_page);
// Determine which page to activate based on URL query parameter "sheet"
const url_params = new URLSearchParams(window.location.search);
const sheet_param = url_params.get("sheet");
let target_page = this.project.first_page;
if (sheet_param) {
// Try to find a matching page
for (const page of this.project.pages()) {

@theacodes theacodes Dec 15, 2025

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Seems like this logic would be useful to move into Project instead of living exclusively in KiCanvasShell, maybe as Project.find_sheet()?

@CapnKernel CapnKernel Dec 16, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good idea, but I don't know TS well enough to take it on. Sorry.

  • If you're able to accept this PR as is, that's great.
  • If you need this change, and you can do it, please do it.
  • If you need this change but you are not in a position to do it, then let's drop this PR.

if (page.page === sheet_param) {
target_page = page;
break;
}
if (
page.name
?.toLowerCase()
.includes(sheet_param.toLowerCase())
) {
target_page = page;
break;
}
if (page.filename === sheet_param) {
target_page = page;
break;
}
}
}
this.project.set_active_page(target_page);
this.loaded = true;
} catch (e) {
console.error(e);
Expand Down