Skip to content

fix: align v0.9 slider schema with spec #1487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kiwigitops wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: align v0.9 slider schema with spec #1487

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
da28ddc
fix: align v0.9 slider schema with spec
kiwigitops May 23, 2026
192683b
test: assert slider step schema error
kiwigitops May 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions renderers/angular/src/v0_9/catalog/basic/slider.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import {SliderApi} from '@a2ui/web_core/v0_9/basic_catalog';
type="range"
[min]="min()"
[max]="max()"
[step]="step()"
[value]="value()"
(input)="handleInput($event)"
class="a2ui-slider"
Expand Down Expand Up @@ -85,7 +84,6 @@ export class SliderComponent extends BasicCatalogComponent<typeof SliderApi> {
readonly value = computed(() => this.props()['value']?.value());
readonly min = computed(() => this.props()['min']?.value() ?? 0);
readonly max = computed(() => this.props()['max']?.value() ?? 100);
readonly step = computed(() => this.props()['step']?.value() ?? 1);

handleInput(event: Event) {
const val = Number((event.target as HTMLInputElement).value);
Expand Down
24 changes: 23 additions & 1 deletion renderers/web_core/src/v0_9/basic_catalog/components/basic_components.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

import {describe, it} from 'node:test';
import * as assert from 'node:assert';
import {ImageApi} from './basic_components.js';
import {ImageApi, SliderApi} from './basic_components.js';

describe('Basic Components Schema', () => {
describe('ImageApi', () => {
Expand Down Expand Up @@ -46,4 +46,26 @@ describe('Basic Components Schema', () => {
assert.throws(() => ImageApi.schema.parse(invalidImage));
});
});

describe('SliderApi', () => {
it('should reject non-spec step property', () => {
const validSlider = {
max: 100,
value: 50,
};
SliderApi.schema.parse(validSlider);

const result = SliderApi.schema.safeParse({
...validSlider,
step: 5,
});

assert.strictEqual(result.success, false);
assert.ok(
result.error.issues.some(
issue => issue.code === 'unrecognized_keys' && issue.keys.includes('step'),
),
);
});
Comment on lines +51 to +69

@gemini-code-assist gemini-code-assist Bot May 23, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The regression test for SliderApi should be more robust. Currently, assert.throws will pass if the schema parsing fails for any reason (e.g., an invalid value format or missing required fields). To ensure it specifically catches the rejection of the step property, it is better to first validate a correct object and then verify that adding step causes a specific error. Additionally, using a literal number for value is more consistent with other tests in this file.

    it('should reject non-spec step property', () => {
      const validSlider = {
        max: 100,
        value: 50,
      };
      // Verify it parses correctly without the 'step' property
      SliderApi.schema.parse(validSlider);

      const invalidSlider = {
        ...validSlider,
        step: 5,
      };

      assert.throws(
        () => SliderApi.schema.parse(invalidSlider),
        /unrecognized_keys.*step/i
      );
    });

});
});
1 change: 0 additions & 1 deletion renderers/web_core/src/v0_9/basic_catalog/components/basic_components.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,7 +442,6 @@ export const SliderApi = {
label: DynamicStringSchema.describe('The label for the slider.').optional(),
min: z.number().default(0).describe('The minimum value of the slider.').optional(),
max: z.number().describe('The maximum value of the slider.'),
step: z.number().describe('The granularity or step size of the slider.').optional(),
value: DynamicNumberSchema.describe('The current value of the slider.'),
checks: CheckableSchema.shape.checks,
})
Expand Down