xuang7 commented on code in PR #5675:
URL: https://github.com/apache/texera/pull/5675#discussion_r3599373348
##########
frontend/src/app/workspace/component/result-panel/result-table-frame/result-table-frame.component.html:
##########
@@ -161,7 +161,40 @@ <h5 class="rightAlign"><span
[innerHTML]="compare(column.header, 'other')"></spa
class="table-cell"
nzEllipsis
(click)="open(i, row)">
- <span class="cell-content">{{ column.getCell(row) }}</span>
+ <span class="cell-content">
+ <ng-container *ngIf="isVideoCell(column.getCell(row)); else
checkAudio">
Review Comment:
Template method calls re-run on every change detection cycle, so each cell
does up to 4 getCell calls plus several regex tests per cycle while the table
is being refreshed during execution. Consider precomputing the media type when
row data arrives, or using a pure pipe.
##########
frontend/src/app/workspace/component/result-panel/result-panel-modal.component.ts:
##########
@@ -42,29 +48,102 @@ import { NgxJsonViewerModule } from "ngx-json-viewer";
selector: "texera-row-modal-content",
templateUrl: "./result-panel-modal.component.html",
styleUrls: ["./result-panel-model.component.scss"],
- imports: [NgxJsonViewerModule],
+ imports: [CommonModule, NzButtonModule, NzIconModule],
})
-export class RowModalComponent implements OnChanges {
+export class RowModalComponent implements OnChanges, OnDestroy {
+ rowEntries: { key: string; value: string; mediaSrc: string; isVideo:
boolean; isImage: boolean; isAudio: boolean }[] =
+ [];
+ private readonly allocatedBlobUrls: string[] = [];
// Index of current displayed row in currentResult
- readonly operatorId: string = inject(NZ_MODAL_DATA).operatorId;
- rowIndex: number = inject(NZ_MODAL_DATA).rowIndex;
+ private readonly modalData: { operatorId: string; rowIndex: number;
rowData?: Record<string, unknown> } =
+ inject(NZ_MODAL_DATA);
+ readonly operatorId: string = this.modalData.operatorId;
+ rowIndex: number = this.modalData.rowIndex;
currentDisplayRowData: Record<string, unknown> = {};
constructor(
public modal: NzModalRef<any, number>,
+ private http: HttpClient,
private workflowResultService: WorkflowResultService,
- private resizeService: PanelResizeService
+ private resizeService: PanelResizeService,
+ private notificationService: NotificationService
) {
+ if (this.modalData.rowData) {
+ this.currentDisplayRowData = this.modalData.rowData;
+ this.rowEntries = this.buildRowEntries(this.currentDisplayRowData);
+ }
this.ngOnChanges();
}
+ get prettyRowJson(): string {
+ return JSON.stringify(this.currentDisplayRowData, null, 2);
+ }
+
+ copyText(text: string): void {
+ navigator.clipboard.writeText(text).then(
+ () => this.notificationService.success("Copied to clipboard"),
+ () => this.notificationService.error("Failed to copy")
+ );
+ }
+
ngOnChanges(): void {
this.workflowResultService
.getPaginatedResultService(this.operatorId)
?.selectTuple(this.rowIndex, this.resizeService.pageSize)
.pipe(untilDestroyed(this))
.subscribe(res => {
- this.currentDisplayRowData = res.tuple;
+ if (res?.tuple) {
+ this.currentDisplayRowData = res.tuple;
+ this.rowEntries = this.buildRowEntries(this.currentDisplayRowData);
+ }
+ });
+ }
+
+ trackByEntryKey(_index: number, entry: { key: string }): string {
+ return entry.key;
+ }
+
+ ngOnDestroy(): void {
+ for (const url of this.allocatedBlobUrls) {
+ URL.revokeObjectURL(url);
+ }
+ }
+
+ private fetchBlobSrc(entry: { mediaSrc: string }, remoteUrl: string): void {
+ const proxyUrl =
`${AppSettings.getApiEndpoint()}/huggingface/media-proxy?url=${encodeURIComponent(remoteUrl)}`;
+ this.http
+ .get(proxyUrl, { responseType: "blob" })
+ .pipe(untilDestroyed(this))
+ .subscribe({
+ next: blob => {
+ const blobUrl = URL.createObjectURL(blob);
+ this.allocatedBlobUrls.push(blobUrl);
+ entry.mediaSrc = blobUrl;
+ },
+ error: () => {
+ entry.mediaSrc = remoteUrl;
Review Comment:
If the backend rejects proxying a media URL (e.g., due to the SSRF
allowlist), the frontend falls back to loading the original URL directly. This
still causes the user's browser to request the remote resource. On a proxy
error, we could set isVideo, isImage, and isAudio to false so the entry falls
back to the existing text view instead of loading the remote URL directly.
--
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]
