Feedbacks from mati (#97) by cy-len · Pull Request #98 · EPFL-ENAC/eesd-mmsdb

cy-len · 2026-03-02T14:33:53Z

Describe your changes

Please explain the purpose and scope of your contribution.

Related GitHub issues and pull requests

Ref: Feedback from Mati #97

Checklist before requesting a review

I have performed a self-review of my code
When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)
Don't forget to link PR to issue if you are solving one.

sphamba

nice

Copilot

Pull request overview

Implements UI/UX and export improvements requested in issue #97, primarily around the Line of Minimum Trace workflow and first-time onboarding.

Changes:

Added computation-history export for Line of Minimum Trace (CSV + PDF) and a “Clear History” action.
Added precise numeric X/Y coordinate inputs alongside sliders in the Line of Minimum Trace page.
Introduced a welcome dialog and adjusted header placement (to address the sticky banner concern) and updated the Home subtitle text.

Reviewed changes

Copilot reviewed 11 out of 13 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
frontend/src/utils/export.ts	New CSV/PDF export helpers for LMT computation traces.
frontend/src/utils/download.ts	Adds helper to download a generated string as a file.
frontend/src/utils/colors.ts	Adds HSL→RGB helpers used for PDF export color rendering.
frontend/src/stores/settings.ts	Adjusts persisted settings default for intro dialog behavior.
frontend/src/pages/LineOfMinimumTracePage.vue	Adds numeric coordinate inputs; adds history export + clear actions; integrates canvas export.
frontend/src/layouts/MainLayout.vue	Moves header into page container and mounts the welcome dialog globally.
frontend/src/components/WelcomeDialog.vue	New onboarding dialog with “Do not show again” toggle tied to settings.
frontend/src/components/LmtCanvas.vue	Exposes canvas image export helpers and refactors redraw flow to support “clean” export.
frontend/src/components/AppToolbar.vue	Removes header rendering from toolbar (now in layout).
frontend/src/components/AppHeader.vue	Updates Home subtitle copy and formatting; removes unused source-count logic.
frontend/package.json	Adds jsPDF and PapaParse (+ types) dependencies.
frontend/package-lock.json	Locks new dependency graph for export feature dependencies.

Files not reviewed (1)

frontend/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-28T22:55:41Z

frontend/src/components/WelcomeDialog.vue

+const showDataProtectionNotice = ref(!!settings.settings?.intro_shown);
+const dontShowAgain = ref(false);
+
+function onClose() {
+  if (dontShowAgain.value) {
+    settings.saveSettings({ intro_shown: false })


showDataProtectionNotice is initialized from settings.settings?.intro_shown, but the setting name reads like a one-time flag while the current behavior is “show unless the user disables it”. Consider renaming the setting (e.g., intro_enabled/show_intro_dialog) or inverting the logic so intro_shown means “has been shown once” (set to true on first display/close) to avoid confusion and future regressions.

Suggested change

const showDataProtectionNotice = ref(!!settings.settings?.intro_shown);

const dontShowAgain = ref(false);

function onClose() {

if (dontShowAgain.value) {

settings.saveSettings({ intro_shown: false })

const showDataProtectionNotice = ref(!settings.settings?.intro_shown);

const dontShowAgain = ref(false);

function onClose() {

if (dontShowAgain.value) {

settings.saveSettings({ intro_shown: true })

Copilot · 2026-03-28T22:55:41Z

frontend/src/stores/settings.ts

    let settingsData: Settings = {
-      intro_shown: false,
+      intro_shown: true,
      theme: 'light',
    };


intro_shown is defaulted to true, which makes the semantics of the flag unclear given its name (it now behaves like “show intro dialog”). To keep the setting self-explanatory and avoid backwards logic across the app, consider either renaming the field or restoring a “not shown yet” default and updating the dialog logic accordingly.

Copilot · 2026-03-28T22:55:42Z

frontend/src/components/LmtCanvas.vue

-  if (!canvasRef.value || !ctx.value || !props.uploadedImage) return;
+function redrawCanvas(withLines: boolean = true) {
+  return new Promise<void>((resolve) => {
+    if (!canvasRef.value || !ctx.value || !props.uploadedImage) return;


redrawCanvas() returns a Promise but if canvasRef/ctx/uploadedImage is missing, the function returns without calling resolve(), leaving callers (e.g., cleanImageDataURL) awaiting a Promise that never settles. Resolve immediately in the early-exit path (or avoid wrapping in a Promise when prerequisites are missing).

Suggested change

if (!canvasRef.value || !ctx.value || !props.uploadedImage) return;

if (!canvasRef.value || !ctx.value || !props.uploadedImage) {

resolve();

return;

}

Copilot · 2026-03-28T22:55:42Z

frontend/src/components/LmtCanvas.vue

+    img.onload = () => {
+      if (ctx.value) {
+        ctx.value.clearRect(0, 0, canvasRef.value!.width, canvasRef.value!.height);
+        ctx.value.drawImage(img, 0, 0);
+
+        if (withLines) {
+          drawLine();
+        }
+        drawResults();
+      }
+
+      resolve();
+    };
+    img.src = URL.createObjectURL(props.uploadedImage);


URL.createObjectURL(props.uploadedImage) is called on every redrawCanvas() (and also initCanvas()) without revoking the created URL, which will leak object URLs over time—especially now that cleanImageDataURL() triggers extra redraws. Store the created URL in a local variable and call URL.revokeObjectURL() after img.onload (or img.onerror).

Suggested change

img.onload = () => {

if (ctx.value) {

ctx.value.clearRect(0, 0, canvasRef.value!.width, canvasRef.value!.height);

ctx.value.drawImage(img, 0, 0);

if (withLines) {

drawLine();

}

drawResults();

}

resolve();

};

img.src = URL.createObjectURL(props.uploadedImage);

const objectUrl = URL.createObjectURL(props.uploadedImage);

img.onload = () => {

try {

if (ctx.value) {

ctx.value.clearRect(0, 0, canvasRef.value!.width, canvasRef.value!.height);

ctx.value.drawImage(img, 0, 0);

if (withLines) {

drawLine();

}

drawResults();

}

} finally {

URL.revokeObjectURL(objectUrl);

resolve();

}

};

img.onerror = () => {

URL.revokeObjectURL(objectUrl);

resolve();

};

img.src = objectUrl;

Copilot · 2026-03-28T22:55:42Z

frontend/src/pages/LineOfMinimumTracePage.vue


+function downloadResults() {
+  const data = traces.value.getAllSuccessValues();
+  downloadStringAsFile(lineComputeTracesToCSV(data), `line_compute_results_${new Date().toISOString()}.csv`, 'text/csv');


The download filenames use new Date().toISOString(), which contains : and can produce invalid filenames on Windows. Use a filename-safe timestamp (e.g., replace : with - or format as YYYYMMDD_HHmmss).

Suggested change

downloadStringAsFile(lineComputeTracesToCSV(data), `line_compute_results_${new Date().toISOString()}.csv`, 'text/csv');

const timestamp = new Date().toISOString().replace(/:/g, '-');

downloadStringAsFile(lineComputeTracesToCSV(data), `line_compute_results_${timestamp}.csv`, 'text/csv');

Copilot · 2026-03-28T22:55:42Z

frontend/src/pages/LineOfMinimumTracePage.vue

+function downloadResults() {
+  const data = traces.value.getAllSuccessValues();
+  downloadStringAsFile(lineComputeTracesToCSV(data), `line_compute_results_${new Date().toISOString()}.csv`, 'text/csv');
+}
+
+async function downloadResultsAsPDF() {
+  const data = traces.value.getAllSuccessValues();
+
+  const dataUrl = (await lmtCanvas.value?.cleanImageDataURL()) || null;
+  void lineComputeTracesToPDF(dataUrl, data, computeLMTResult);
+}


This code exports whatever getAllSuccessValues() returns, but the history UI explicitly handles value.result.success === false. If failed computations can be present in the exported array, lineComputeTracesToCSV/PDF will throw when accessing fields like path_coordinates. Consider filtering to trace.result.success === true before exporting or make the export functions resilient (include error rows with empty result fields).

Copilot · 2026-03-28T22:55:42Z

frontend/src/utils/export.ts

+
+    });
+
+    return doc.save(`line_compute_results_${new Date().toISOString()}.pdf`);


The PDF filename uses new Date().toISOString(), which includes : and can yield invalid filenames on Windows downloads. Prefer a filename-safe timestamp.

Suggested change

return doc.save(`line_compute_results_${new Date().toISOString()}.pdf`);

const timestamp = new Date().toISOString().replace(/[:]/g, '-');

return doc.save(`line_compute_results_${timestamp}.pdf`);

Copilot · 2026-03-28T22:55:43Z

frontend/src/utils/export.ts

+    const data = lines.map(line => ({
+        // inputs
+        start_x: line.params.startX,
+        start_y: line.params.startY,
+        end_x: line.params.endX,
+        end_y: line.params.endY,
+        real_length: line.params.realLength,
+        real_height: line.params.realHeight,
+        interface_weight: line.params.interfaceWeight,
+        boundary_margin: line.params.boundaryMargin,
+
+        // results
+        lmt_type: line.result.lmt_type,
+        lmt_result: line.result.lmt_result,
+        total_length: line.result.total_length,
+        start_point_used_x: line.result.start_point_used[0],
+        start_point_used_y: line.result.start_point_used[1],
+        end_point_used_x: line.result.end_point_used[0],
+        end_point_used_y: line.result.end_point_used[1],
+        path: line.result.path_coordinates.pixel_coordinates.map(coord => `${coord[0]}_${coord[1]}`).join("|"),
+    }));
+
+    return Papa.unparse(data, {
+        header: true,
+    });
+}
+
+export function lineComputeTracesToPDF(imageDataUrl: string | null, lines: LineComputeTrace[], extractResult: (line: LineComputeTrace) => number | null = line => line.result.lmt_result) {


lineComputeTracesToCSV() assumes all result fields exist (e.g., start_point_used, path_coordinates). However, the UI elsewhere accounts for result.success === false with an error-only payload. Either filter to successful results before building CSV/PDF rows or guard accesses and export error information when success is false.

Suggested change

const data = lines.map(line => ({

// inputs

start_x: line.params.startX,

start_y: line.params.startY,

end_x: line.params.endX,

end_y: line.params.endY,

real_length: line.params.realLength,

real_height: line.params.realHeight,

interface_weight: line.params.interfaceWeight,

boundary_margin: line.params.boundaryMargin,

// results

lmt_type: line.result.lmt_type,

lmt_result: line.result.lmt_result,

total_length: line.result.total_length,

start_point_used_x: line.result.start_point_used[0],

start_point_used_y: line.result.start_point_used[1],

end_point_used_x: line.result.end_point_used[0],

end_point_used_y: line.result.end_point_used[1],

path: line.result.path_coordinates.pixel_coordinates.map(coord => `${coord[0]}_${coord[1]}`).join("|"),

}));

return Papa.unparse(data, {

header: true,

});

}

export function lineComputeTracesToPDF(imageDataUrl: string | null, lines: LineComputeTrace[], extractResult: (line: LineComputeTrace) => number | null = line => line.result.lmt_result) {

const data = lines.map(line => {

const base = {

// inputs

start_x: line.params.startX,

start_y: line.params.startY,

end_x: line.params.endX,

end_y: line.params.endY,

real_length: line.params.realLength,

real_height: line.params.realHeight,

interface_weight: line.params.interfaceWeight,

boundary_margin: line.params.boundaryMargin,

};

// If this line has an error-only result payload, avoid accessing

// fields that may not exist and instead export serialized error info.

const resultAny = line.result as any;

if (resultAny && resultAny.success === false) {

return {

...base,

// results (empty because computation failed)

lmt_type: "",

lmt_result: "",

total_length: "",

start_point_used_x: "",

start_point_used_y: "",

end_point_used_x: "",

end_point_used_y: "",

path: "",

error: JSON.stringify(resultAny),

};

}

const pathCoordinates = (line.result as any)?.path_coordinates;

const pixelCoordinates: unknown = pathCoordinates?.pixel_coordinates;

const path =

Array.isArray(pixelCoordinates)

? (pixelCoordinates as [number, number][])

.map(coord => `${coord[0]}_${coord[1]}`)

.join("|")

: "";

return {

...base,

// results

lmt_type: line.result.lmt_type,

lmt_result: line.result.lmt_result,

total_length: line.result.total_length,

start_point_used_x: line.result.start_point_used[0],

start_point_used_y: line.result.start_point_used[1],

end_point_used_x: line.result.end_point_used[0],

end_point_used_y: line.result.end_point_used[1],

path,

};

});

return Papa.unparse(data, {

header: true,

});

}

export function lineComputeTracesToPDF(

imageDataUrl: string | null,

lines: LineComputeTrace[],

extractResult: (line: LineComputeTrace) => number | null = line => {

const resultAny = line.result as any;

if (resultAny && resultAny.success === false) {

return null;

}

const value = (line.result as any).lmt_result;

return typeof value === "number" ? value : null;

},

) {

feat: export LMT computation history in csv and pdf

bfac676

cy-len requested review from Copilot and sphamba and removed request for Copilot March 2, 2026 14:33

Copilot started reviewing on behalf of cy-len March 2, 2026 14:34 View session

sphamba approved these changes Mar 2, 2026

View reviewed changes

feat: last requests from mati

b16764d

cy-len requested review from Copilot and sphamba March 28, 2026 22:50

Copilot started reviewing on behalf of cy-len March 28, 2026 22:50 View session

Copilot AI reviewed Mar 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feedbacks from mati (#97)#98

Feedbacks from mati (#97)#98
cy-len wants to merge 2 commits intodevfrom
feat/mati-feedbacks

cy-len commented Mar 2, 2026

Uh oh!

sphamba left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Copilot AI Mar 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	downloadStringAsFile(lineComputeTracesToCSV(data), `line_compute_results_${new Date().toISOString()}.csv`, 'text/csv');
	const timestamp = new Date().toISOString().replace(/:/g, '-');
	downloadStringAsFile(lineComputeTracesToCSV(data), `line_compute_results_${timestamp}.csv`, 'text/csv');


		});

		return doc.save(`line_compute_results_${new Date().toISOString()}.pdf`);

Conversation

cy-len commented Mar 2, 2026

Describe your changes

Related GitHub issues and pull requests

Checklist before requesting a review

Uh oh!

sphamba left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants