Copilot commented on code in PR #6263:
URL: https://github.com/apache/texera/pull/6263#discussion_r3540616748
##########
frontend/src/app/common/component/computing-unit-create-modal/computing-unit-create-modal.component.ts:
##########
@@ -229,18 +234,20 @@ export class ComputingUnitCreateModalComponent implements
OnInit {
this.resetJvmMemorySlider();
}
- onJvmMemorySliderChange(value: number): void {
+ onJvmMemorySliderChange(index: number): void {
// Ensure the value is one of the valid steps
- const validStep = findNearestValidStep(value, this.jvmMemorySteps);
- this.jvmMemorySliderValue = validStep;
- this.selectedJvmMemorySize = `${validStep}G`;
+ const validStep = this.jvmMemorySteps[index];
+ if (validStep !== undefined) {
+ this.jvmMemorySliderValue = index;
+ this.selectedJvmMemorySize = `${validStep}G`;
+ }
}
Review Comment:
Existing unit tests in computing-unit-create-modal.component.spec.ts still
treat `jvmMemorySliderValue` as a GB value and call `onJvmMemorySliderChange()`
with step values (e.g., `component.jvmMemorySteps[0]`, `4`). After this change,
the slider model is an *index*, so those tests (and any other callers) should
be updated to use indices and assert against the mapped step
(`jvmMemorySteps[index]`), otherwise the test suite will fail and/or assertions
will be incorrect.
##########
frontend/src/app/common/util/computing-unit.util.ts:
##########
@@ -299,23 +291,23 @@ export function getJvmMemorySliderConfig(selectedMemory:
string): JvmMemorySlide
const steps = buildJvmMemorySteps(cuMemoryInGb, 1);
return {
- jvmMemoryMax: cuMemoryInGb,
+ jvmMemoryMax: steps.length - 1,
showJvmMemorySlider: false,
jvmMemorySteps: steps,
jvmMemoryMarks: buildJvmMemoryMarks(steps),
- jvmMemorySliderValue: defaultValue,
+ jvmMemorySliderValue: steps.indexOf(defaultValue),
selectedJvmMemorySize: `${defaultValue}G`,
};
}
const steps = buildJvmMemorySteps(cuMemoryInGb, 2);
return {
- jvmMemoryMax: cuMemoryInGb,
+ jvmMemoryMax: steps.length - 1,
showJvmMemorySlider: true,
jvmMemorySteps: steps,
jvmMemoryMarks: buildJvmMemoryMarks(steps),
- jvmMemorySliderValue: 2,
+ jvmMemorySliderValue: steps.indexOf(2) !== -1 ? steps.indexOf(2) : 0,
selectedJvmMemorySize: "2G",
Review Comment:
`steps.indexOf(2)` is computed twice. This is minor, but it’s easy to
simplify while keeping the same fallback behavior when `2` isn’t present.
--
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]
