Yicong-Huang commented on code in PR #6213: URL: https://github.com/apache/texera/pull/6213#discussion_r3890371277
########## frontend/src/app/workspace/service/heatmap/heatmap-color.ts: ########## @@ -0,0 +1,63 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +type Rgb = readonly [number, number, number]; + +/** + * Cold -> hot ramp (ColorBrewer RdYlBu, reversed): blue -> pale yellow -> red. + * Chosen because it is colorblind-safer than a rainbow ramp. + */ +const COLD: Rgb = [91, 155, 213]; // #5b9bd5 — bright blue (still light enough for readable labels) +const MID: Rgb = [255, 255, 191]; // #ffffbf — pale yellow +const HOT: Rgb = [224, 90, 82]; // #e05a52 — bright red + +/** Neutral fill for an operator that has no score (no metrics captured yet). */ Review Comment: `999e096f8` fixed this same half-story at the constant's only consumer (`joint-ui.service.ts:522-523`), but the declaration still says grey means only "no metrics captured yet". Since round 4, `rawMetricForView` also returns undefined for an operator that *has* metrics but is not measurable in the active view, and this constant paints both. Same shape at `workflow-editor.component.ts:534-535`: the tooltip comment credits "missing metrics" for both `"—"` fields, but the not-measurable case renders `"—"` too. ```suggestion /** * Neutral fill for an operator with no score — either no metrics captured yet, or the active * view is not measurable for it. */ ``` ########## frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts: ########## @@ -515,6 +516,25 @@ export class JointUIService { jointPaper.getModelById(operator.operatorID).attr("rect.body/fill", JointUIService.getOperatorFillColor(operator)); } + /** + * Paints an operator's body fill for the performance heat-map overlay. The heat-map owns only + * `rect.body/fill`, so it coexists with the execution-status border (`rect.body/stroke`). + * A `score` of undefined means no heat is known for the operator — either no metrics + * captured yet, or the active view is not measurable for it — and paints a neutral color. Review Comment: Small snag from the new aside: the subject of both verbs is "A `score` of undefined", so the sentence reads as the score being what paints. That was easy to miss before, when "and paints" sat close to "means"; the 14-word aside now separates them by a full line. ```suggestion * The method paints a neutral color when `score` is undefined, which means no heat is known * for the operator — either no metrics captured yet, or the active view is not measurable * for it. ``` -- 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]
