yangzhang75 commented on code in PR #8441:
URL: https://github.com/apache/texera/pull/8441#discussion_r3945966021
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -141,5 +221,70 @@
class="box"></texera-mini-map>
</div>
</section>
+
+ <!-- Results, under the workflow that produced them. The section keeps its
place before a run so
+ the answer has a visible destination. -->
+ <section class="results">
+ <div class="pc-section-head"><span class="label">Results</span></div>
+ <p
+ class="results-empty"
+ *ngIf="resultIdsToShow.length === 0">
+ {{ isRunning ? "Working…" : "Press Run and the results appear here." }}
Review Comment:
Fixed. Added a hasRunFinished getter (Completed/Failed/Killed/Terminated);
the empty results line now reads "This run produced no results to show." after
a finished run, keeping "Press Run..." only for a form that has not run yet.
Added a test.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.html:
##########
@@ -141,5 +221,70 @@
class="box"></texera-mini-map>
</div>
</section>
+
+ <!-- Results, under the workflow that produced them. The section keeps its
place before a run so
+ the answer has a visible destination. -->
+ <section class="results">
+ <div class="pc-section-head"><span class="label">Results</span></div>
+ <p
+ class="results-empty"
+ *ngIf="resultIdsToShow.length === 0">
+ {{ isRunning ? "Working…" : "Press Run and the results appear here." }}
+ </p>
+ <!-- A card only for a chosen step that actually produced a result.
Whether a Python UDF
+ yields one cannot be told from the graph, so a step earns its card
at runtime rather than
+ sitting on a permanent "No result yet." (a download/publish step
never would). -->
+ <ng-container *ngFor="let id of resultIdsToShow">
+ <div class="card result">
+ <div class="result-head">
+ <span>{{ resultLabel(id) }}</span>
+ <!-- Zooming only means something once there is a picture to zoom.
-->
+ <span
+ class="result-zoom"
+ *ngIf="vizHasContent(id)">
+ <button
+ (click)="zoomResult(id, -1)"
+ [disabled]="resultZoom(id) <= 0"
+ nz-tooltip="Smaller">
Review Comment:
Fixed (this thread is anchored to an older commit): the zoom buttons now
carry aria-label "Smaller"/"Bigger" and their icons are aria-hidden, so screen
readers announce a named button.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -161,8 +228,95 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return;
}
this.wid = wid;
+ // Give the result tables a realistic height to page against, so they show
a screenful of rows
+ // instead of one. (~7 rows; the card scrolls for the rest.)
+ this.panelResizeService.changePanelSize(900, 560);
this.load(wid);
+ // A result changing bumps that operator's version (so its chart frame is
rebuilt, not reused),
+ // re-limits what the form shows to the currently-viewed set, and re-fits
the visualisations.
+ this.workflowResultService
+ .getResultUpdateStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(update => {
+ for (const operatorID of Object.keys(update ?? {})) {
+ this.resultVersion.set(operatorID,
(this.resultVersion.get(operatorID) ?? 0) + 1);
+ }
+ this.refreshShownResults();
Review Comment:
Fixed: the component now also subscribes to
getViewResultOperatorsChangedStream() and re-runs refreshShownResults on it, so
a view-result change that emits no result update (a co-editor turning the eye
off, or a delete) drops the card at once. Added a test. (This thread points at
the result-update subscription; the new one sits right below it.)
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -161,8 +228,95 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return;
}
this.wid = wid;
+ // Give the result tables a realistic height to page against, so they show
a screenful of rows
+ // instead of one. (~7 rows; the card scrolls for the rest.)
+ this.panelResizeService.changePanelSize(900, 560);
this.load(wid);
+ // A result changing bumps that operator's version (so its chart frame is
rebuilt, not reused),
+ // re-limits what the form shows to the currently-viewed set, and re-fits
the visualisations.
+ this.workflowResultService
+ .getResultUpdateStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(update => {
+ for (const operatorID of Object.keys(update ?? {})) {
+ this.resultVersion.set(operatorID,
(this.resultVersion.get(operatorID) ?? 0) + 1);
+ }
+ this.refreshShownResults();
+ this.cdr.detectChanges();
+ this.later(() => this.fitVisualisations(), 300);
+ });
+
+ // The run clock, reusing the operator canvas's source outright rather
than timing anything
+ // here: the engine is the only thing that knows when the run really
began, so a stopwatch
+ // started at the click would drift and would be wrong after a reload.
+ this.workflowWebsocketService
+ .subscribeToEvent("ExecutionDurationUpdateEvent")
+ .pipe(
+ tap(event => (this.executionDuration = event.duration)),
+ switchMap(event => (event.isRunning ? timer(1000, 1000) : EMPTY)),
+ untilDestroyed(this)
+ )
+ .subscribe(() => {
+ this.executionDuration += 1000;
+ this.cdr.markForCheck();
+ });
+
+ // The run button's state is read from getters, so a change in
unit/connection/validity has to
+ // repaint the view. markForCheck, not detectChanges: a synchronous pass
can be thrown out of by
+ // an unrelated component's NG0100, killing the subscription.
+ this.computingUnitStatusService
+ .getSelectedComputingUnit()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.cdr.markForCheck());
+ this.computingUnitStatusService
+ .getStatus()
+ .pipe(untilDestroyed(this))
+ .subscribe(status => {
+ this.computingUnitStatus = status;
+ this.cdr.markForCheck();
+ });
+ this.workflowWebsocketService
+ .getConnectionStatusStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.cdr.markForCheck());
+ // Validity from the canvas's own stream, so a broken graph disables Run
("Invalid") here
+ // exactly as it does there.
+ this.validationWorkflowService
+ .getWorkflowValidationErrorStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(value => {
+ this.isWorkflowEmpty = value.workflowEmpty;
+ this.isWorkflowValid = Object.keys(value.errors).length === 0;
+ this.cdr.markForCheck();
+ });
+
+ this.executeWorkflowService
+ .getExecutionStateStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(({ current }) => {
+ this.executionState = current.state;
+ // Surface a failed run. Without this the spinner just stops and the
form gives zero
+ // feedback -- the opposite of what a reader needs. onRun() clears
runError before the next
+ // run, so a stale error never lingers.
+ if (current.state === ExecutionState.Failed) {
+ // A required input left empty is by far the commonest reason a run
fails here, and the
+ // engine reports it as an opaque "... is not contained in the
schema". Answer with the
+ // same word the field itself already shows ("required"), so the two
messages are
+ // consistent -- and it covers every operator, not just this one.
+ this.runError = this.hasEmptyRequiredInputs()
+ ? "Run failed: please fill in the required fields."
+ :
this.friendlyRunError(current.errorMessages?.[0]?.message?.trim() ?? "");
+ }
Review Comment:
Fixed: the execution-state handler now clears runError the moment a new run
starts (wasRunning false -> true), so a co-editor's next run no longer carries
the previous failure banner. Added a test.
##########
frontend/src/app/workspace/component/workflow-form/workflow-form.component.ts:
##########
@@ -401,6 +681,268 @@ export class WorkflowFormComponent implements OnInit,
OnDestroy {
return rendered.resolved.binding.id;
}
+ //
---------------------------------------------------------------------------
+ // Results: the chosen steps' output, shown under the workflow that produced
it
+ //
---------------------------------------------------------------------------
+
+ public get hasResults(): boolean {
+ return this.shownResultIds.some(id =>
this.workflowResultService.hasNonEmptyResult(id));
+ }
+
+ /**
+ * The chosen steps that actually produced a result, so only those get a
card. Whether a Python
+ * UDF yields a result cannot be known from the graph -- some (a
download/publish step) never do --
+ * so a chosen step earns its card at runtime rather than sitting on a
permanent "No result yet.".
+ */
+ public get resultIdsToShow(): string[] {
+ return this.shownResultIds.filter(id =>
this.workflowResultService.hasNonEmptyResult(id));
+ }
+
+ public isTabularResult(operatorID: string): boolean {
+ return this.workflowResultService.hasPaginatedResult(operatorID);
+ }
+
+ /**
+ * Whether this step's visualisation drew something. A visualiser reserves a
fixed canvas even
+ * when empty, so gating on real content lets an empty result collapse to
the compact "No result
+ * yet" line instead of a tall blank box. Tables are excluded (they take the
tabular branch).
+ */
+ public vizHasContent(operatorID: string): boolean {
+ if (this.isTabularResult(operatorID)) {
+ return false;
+ }
+ const snapshot =
this.workflowResultService.getResultService(operatorID)?.getCurrentResultSnapshot();
+ return !!snapshot && snapshot.length > 0;
+ }
+
+ /** The operator's friendly label for a result card, falling back to its raw
id. */
+ public resultLabel(operatorID: string): string {
+ const operator =
this.workflowActionService.getTexeraGraph().getOperator(operatorID);
+ return operator ? this.formBindingService.operatorLabel(operator) :
operatorID;
+ }
+
+ public trackByKey(_: number, key: string): string {
+ return key;
+ }
+
+ /**
+ * A per-result identity that changes only when that operator's result is
genuinely new, used as
+ * the chart's *ngFor key so the frame is rebuilt (not reused) on a new
result: the chart reads
+ * its content once at creation, so a reused frame kept showing the old (or
"undefined") picture.
+ */
+ public resultKey(operatorID: string): string {
+ return operatorID + "#" + (this.resultVersion.get(operatorID) ?? 0);
+ }
+
+ public resultZoom(operatorID: string): number {
+ return this.zoomByResult.get(operatorID) ?? 1;
+ }
+
+ public zoomResult(operatorID: string, delta: number): void {
+ const next = Math.min(2, Math.max(0, this.resultZoom(operatorID) + delta));
+ this.zoomByResult.set(operatorID, next);
+ // Let the new card height land, then have the chart redraw into it --
growing the frame alone
+ // leaves the picture at its old size until something asks it to
re-measure.
+ this.cdr.detectChanges();
+ this.later(() => this.fitVisualisations(), 60);
+ }
+
+ /**
+ * Scale each visualisation to its card. They render in a same-origin srcdoc
iframe at natural
+ * size, so we inject a stylesheet to fit the content to the card and fire a
resize so chart
+ * libraries re-lay out. The operator's output is untouched.
+ */
+ /* v8 ignore start -- iframe/Plotly DOM fitting; no coverage in jsdom */
+ private fitVisualisations(): void {
+ const frames =
this.host.nativeElement.querySelectorAll<HTMLIFrameElement>(".result-body
iframe");
+ frames.forEach(frame => {
+ const apply = () => {
+ try {
+ const doc = frame.contentDocument;
+ if (!doc?.body) {
+ return;
+ }
+ if (!doc.getElementById("pc-fit")) {
+ const style = doc.createElement("style");
+ style.id = "pc-fit";
+ style.textContent = `
+ html, body { margin: 0; padding: 8px; overflow-x: hidden; }
+ .js-plotly-plot, .plot-container, .plotly, .svg-container {
width: 100% !important; height: 100% !important; }
+ img, svg, canvas, video { max-width: 100% !important; height:
auto !important; }
+ table { max-width: 100%; }
+ `;
+ doc.head?.appendChild(style);
+ }
+ const win = frame.contentWindow as (Window & { Plotly?: any }) |
null;
+ const plots = doc.querySelectorAll<HTMLElement>(".js-plotly-plot");
+ if (win?.Plotly?.Plots?.resize && plots.length) {
+ plots.forEach(plot => {
+ plot.style.width = "100%";
+ plot.style.height = "100%";
+ try {
+ win.Plotly.Plots.resize(plot);
+ } catch {
+ // A chart mid-render cannot be resized; the next call will
catch it.
+ }
+ });
+ }
+ win?.dispatchEvent(new Event("resize"));
+ } catch {
+ // A cross-origin document cannot be styled from here; leave it as
it came.
+ }
+ };
+ apply();
+ frame.addEventListener("load", apply, { once: true });
Review Comment:
Fixed: the load listener is now bound once per frame element (guarded by a
data flag) instead of a fresh { once: true } listener on every fit call, so
repeated zoom/fit no longer piles up detached listeners that never fire on an
already-loaded iframe.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
