Yicong-Huang commented on code in PR #6437:
URL: https://github.com/apache/texera/pull/6437#discussion_r3647413155
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1712,6 +1728,146 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
return
this.operatorSummaries.get(operatorId)?.sampleRecords?.[0]?.["__is_visualization__"]
=== true;
}
+ /**
+ * Ambient operator recommender (apache/texera#5240). When an operator is
+ * added, ask the recommender for likely next operators and float them as
+ * suggestion chips on the operator's output port; clicking one materializes
+ * it. The whole feature is opt-in and self-effacing: if it is disabled or
the
+ * backend returns nothing, the canvas is untouched.
+ */
+ private handleOperatorRecommendation(): void {
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // Trigger: an operator was just added to the canvas.
+ this.workflowActionService
+ .getTexeraGraph()
+ .getOperatorAddStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(operator => this.showRecommendationsFor(operator));
+
+ // Dismiss when the user clicks on blank canvas.
+ fromJointPaperEvent(this.paper, "blank:pointerdown")
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.closeRecommendations());
+
+ // Dismiss if the anchor operator is deleted out from under the
suggestions.
+ this.workflowActionService
+ .getTexeraGraph()
+ .getOperatorDeleteStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(({ deletedOperatorID }) => {
+ if (this.operatorSuggestion?.operatorId === deletedOperatorID) {
+ this.closeRecommendations();
+ }
+ });
+
+ // Keep the suggestions anchored to the operator's output port as it moves.
+ this.paper.model.on("change:position", (cell: joint.dia.Cell) => {
+ if (this.operatorSuggestion && cell.id.toString() ===
this.operatorSuggestion.operatorId) {
+ this.repositionRecommendations();
+ }
+ });
+
+ // Keep the suggestions anchored on zoom / pan.
+ this.wrapper
+ .getWorkflowEditorZoomStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => {
+ if (this.operatorSuggestion) {
+ this.repositionRecommendations();
+ }
+ });
+ }
+
+ private showRecommendationsFor(operator: OperatorPredicate): void {
+ this.closeRecommendations();
+ if (operator.outputPorts.length === 0) {
+ return;
+ }
+ const sourceOutputPortID = operator.outputPorts[0].portID;
+
+ this.operatorRecommendationService
+ .getRecommendations(operator)
+ .pipe(untilDestroyed(this))
+ .subscribe(recommendations => {
+ // The operator may have been deleted while the request was in flight.
+ if (
+ recommendations.length === 0 ||
+
!this.workflowActionService.getTexeraGraph().hasOperator(operator.operatorID)
+ ) {
+ return;
+ }
+ const position = this.getRecommendationPosition(operator.operatorID);
+ if (!position) {
+ return;
+ }
+ this.operatorSuggestion = {
+ operatorId: operator.operatorID,
+ sourceOutputPortID,
+ position,
+ recommendations,
+ };
+ this.changeDetectorRef.detectChanges();
+ });
+ }
+
+ /**
+ * Materialize a clicked suggestion into a real operator wired onto the
+ * source operator's output port.
+ */
+ materializeRecommendation(recommendation: OperatorRecommendation): void {
+ if (!this.operatorSuggestion) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ if (graph.hasOperator(this.operatorSuggestion.operatorId)) {
+ const sourceOperator =
graph.getOperator(this.operatorSuggestion.operatorId);
+ this.operatorRecommendationService.materialize(
+ sourceOperator,
+ this.operatorSuggestion.sourceOutputPortID,
+ recommendation.operatorType
+ );
+ }
+ this.closeRecommendations();
+ }
+
+ closeRecommendations(): void {
+ if (this.operatorSuggestion) {
+ this.operatorSuggestion = null;
+ this.changeDetectorRef.detectChanges();
+ }
+ }
+
+ private repositionRecommendations(): void {
+ if (!this.operatorSuggestion) {
+ return;
+ }
+ const position =
this.getRecommendationPosition(this.operatorSuggestion.operatorId);
+ if (position) {
+ this.operatorSuggestion = { ...this.operatorSuggestion, position };
+ }
+ this.changeDetectorRef.detectChanges();
Review Comment:
for angular, it uses `this.changeDetectorRef.detectChanges();` and when that
happens, it recalculates all the pixels and re-renders the frontend components.
I think @xuang7 's point is that such re-render might be too frequent. it
will be laggy or blinky on the UX. She suggested for
- early return (do not invoke `detectChanges()`) if position has not
changed: so that no re-render if everything would stay the same.
- debouncing: do not rerender right away, after you get a few frames of
changes, render them all at once. the debouncing could be controlled by number
of invocations here (i.e., when entering this function 30 times), or by time
(i.e., every 200/300 ms).
I think both suggestions are valid, if the UX is really bad. @gupta-sahil01
does that answer your question?
--
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]
