feat(genui): add A2UI Slider component#2663
Conversation
MoonfaceX
requested review from
HuJean,
PupilTong,
Sherry-hue,
fzx2666-fz and
gaoachao
as code owners
🦋 Changeset detectedLatest commit: a105197 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (9)
📝 WalkthroughWalkthroughA new Slider input component is added to the A2UI catalog with utilities for normalization and ratio conversion, a React implementation with validation, CSS styling, unit tests, package export/re-exports, playground registration, and documentation and changelog entries. ChangesSlider Component Addition
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/genui/a2ui/src/catalog/Slider/index.tsx (1)
92-95: ⚡ Quick winType the dynamic slider properties or document their absence.
The component accesses
props['minValue'],props['maxValue'], andprops['step']using bracket notation, but these properties are not declared in theSliderPropsinterface. This creates a typing gap where the component expects properties that aren't part of its contract.📝 Option 1: Add optional properties to SliderProps
export interface SliderProps extends GenericComponentProps { /** The label for the slider. */ label?: string | { path: string } | { ... }; /** The minimum value of the slider. */ min?: number; + /** Alternative property name for minimum value. */ + minValue?: number; /** The maximum value of the slider. */ max: number; + /** Alternative property name for maximum value. */ + maxValue?: number; + /** The step increment for discrete value changes. */ + step?: number; /** The current value of the slider. */ value: number | { path: string } | { ... };📝 Option 2: Use type assertion if these are intentionally dynamic
If
minValue,maxValue, andstepare meant to be passed dynamically (outside the typed interface), add a comment explaining this pattern to help future maintainers understand the design choice.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/genui/a2ui/src/catalog/Slider/index.tsx` around lines 92 - 95, The Slider component is reading props['minValue'], props['maxValue'], and props['step'] but those keys are not declared on the SliderProps interface; update the type contract to prevent the typing gap by adding optional numeric properties minValue?: number, maxValue?: number, and step?: number to SliderProps (or, if these values are intentionally dynamic, add an explicit index signature or a clear comment and use a narrow type assertion at the usage site) so the usages in the Slider component (around normalizeSliderRange and normalizeSliderNumber) are properly typed and documented.packages/genui/a2ui/styles/catalog/Slider.css (1)
36-37: 💤 Low valueConsider using CSS variable for consistent spacing.
The slider control uses hardcoded
12pxmargins while other spacing in this file uses CSS variables likevar(--a2ui-spacing-s)(line 6) andvar(--a2ui-spacing-m)(line 16). Using a variable would maintain consistency with the theming system.♻️ Proposed change
.slider-control { - margin-left: 12px; - margin-right: 12px; + margin-left: var(--a2ui-spacing-m); + margin-right: var(--a2ui-spacing-m); }Note: Verify that
--a2ui-spacing-mequals 12px before applying, or use the appropriate variable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/genui/a2ui/styles/catalog/Slider.css` around lines 36 - 37, Replace the hardcoded "12px" margins in the Slider.css rule with the project spacing CSS variable to keep theming consistent: change margin-left and margin-right to use the appropriate variable (e.g., var(--a2ui-spacing-m) or var(--a2ui-spacing-s) if that matches 12px) within the same selector in Slider.css (ensure you verify which variable equals 12px before choosing).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.changeset/petite-coins-warn.md:
- Around line 1-5: The changeset frontmatter is empty; add a package entry and
bump type between the leading and trailing `---` so the changeset targets the
package (for example add `"`@lynx-js/a2ui-reactlynx`": patch`) — update the
frontmatter in .changeset/petite-coins-warn.md to include the package name and
bump level matching this public feature addition.
In `@packages/genui/a2ui/src/catalog/Slider/index.tsx`:
- Around line 99-116: The displayValue state (initialized via useState and
updated only in handleValueChange) can get out of sync when props.value/ratio
changes externally; add a useEffect that watches the external ratio (or
props.value) and calls setDisplayValue(Math.round(fromSliderRatio(ratio, range,
step))) to keep the displayed number synchronized with the slider thumb; import
useEffect from `@lynx-js/react` and update the effect dependencies to include
ratio, range, and step so displayValue updates whenever the external value
changes.
---
Nitpick comments:
In `@packages/genui/a2ui/src/catalog/Slider/index.tsx`:
- Around line 92-95: The Slider component is reading props['minValue'],
props['maxValue'], and props['step'] but those keys are not declared on the
SliderProps interface; update the type contract to prevent the typing gap by
adding optional numeric properties minValue?: number, maxValue?: number, and
step?: number to SliderProps (or, if these values are intentionally dynamic, add
an explicit index signature or a clear comment and use a narrow type assertion
at the usage site) so the usages in the Slider component (around
normalizeSliderRange and normalizeSliderNumber) are properly typed and
documented.
In `@packages/genui/a2ui/styles/catalog/Slider.css`:
- Around line 36-37: Replace the hardcoded "12px" margins in the Slider.css rule
with the project spacing CSS variable to keep theming consistent: change
margin-left and margin-right to use the appropriate variable (e.g.,
var(--a2ui-spacing-m) or var(--a2ui-spacing-s) if that matches 12px) within the
same selector in Slider.css (ensure you verify which variable equals 12px before
choosing).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 444ee772-cfb6-4d3b-8131-55244dfe3031
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.changeset/petite-coins-warn.mdpackages/genui/a2ui-playground/lynx-src/a2ui/App.tsxpackages/genui/a2ui-playground/src/catalog/a2ui.tspackages/genui/a2ui/README.mdpackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/README.mdpackages/genui/a2ui/src/catalog/Slider/index.tsxpackages/genui/a2ui/src/catalog/Slider/utils.tspackages/genui/a2ui/src/catalog/index.tspackages/genui/a2ui/src/index.tspackages/genui/a2ui/styles/catalog/Slider.csspackages/genui/a2ui/test/slider.test.ts
MoonfaceX
force-pushed
the
p/xiamengfei.moonface/a2ui-slider
branch
from
7b45364 to
27b450e
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/genui/a2ui/package.json (1)
127-136:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove stale
@lynx-js/lynx-ui-inputfrompeerDependenciesMeta.Lines 134-136 declare
@lynx-js/lynx-ui-inputas an optional peer dependency, but it is neither listed inpeerDependenciesnor imported anywhere in this package. Remove the stale entry.Diff
"peerDependenciesMeta": { "`@lynx-js/lynx-ui`": { "optional": true }, - "`@lynx-js/lynx-ui-input`": { - "optional": true - } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/genui/a2ui/package.json` around lines 127 - 136, Remove the stale peerDependenciesMeta entry for "`@lynx-js/lynx-ui-input`" from package.json: locate the "peerDependenciesMeta" object and delete the entire block referencing "`@lynx-js/lynx-ui-input`" so only actual peer dependencies (e.g., "`@lynx-js/lynx-ui`") remain; ensure no other references to "`@lynx-js/lynx-ui-input`" exist in the file after removal.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@packages/genui/a2ui/package.json`:
- Around line 127-136: Remove the stale peerDependenciesMeta entry for
"`@lynx-js/lynx-ui-input`" from package.json: locate the "peerDependenciesMeta"
object and delete the entire block referencing "`@lynx-js/lynx-ui-input`" so only
actual peer dependencies (e.g., "`@lynx-js/lynx-ui`") remain; ensure no other
references to "`@lynx-js/lynx-ui-input`" exist in the file after removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 31198046-166d-4ed0-bcc8-bb48b301dee1
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
.changeset/petite-coins-warn.mdpackages/genui/a2ui-playground/lynx-src/a2ui/App.tsxpackages/genui/a2ui-playground/src/catalog/a2ui.tspackages/genui/a2ui/README.mdpackages/genui/a2ui/package.jsonpackages/genui/a2ui/src/catalog/README.mdpackages/genui/a2ui/src/catalog/Slider/index.tsxpackages/genui/a2ui/src/catalog/Slider/utils.tspackages/genui/a2ui/src/catalog/index.tspackages/genui/a2ui/src/index.tspackages/genui/a2ui/styles/catalog/Slider.csspackages/genui/a2ui/test/slider.test.ts
✅ Files skipped from review due to trivial changes (2)
- packages/genui/a2ui/src/catalog/README.md
- .changeset/petite-coins-warn.md
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/genui/a2ui/src/catalog/index.ts
- packages/genui/a2ui/src/index.ts
- packages/genui/a2ui/test/slider.test.ts
- packages/genui/a2ui-playground/src/catalog/a2ui.ts
- packages/genui/a2ui/README.md
- packages/genui/a2ui-playground/lynx-src/a2ui/App.tsx
- packages/genui/a2ui/styles/catalog/Slider.css
- packages/genui/a2ui/src/catalog/Slider/utils.ts
- packages/genui/a2ui/src/catalog/Slider/index.tsx
Merging this PR will degrade performance by 21.03%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | 002-hello-reactLynx-destroyBackground |
669.8 µs | 916.7 µs | -26.93% |
| ❌ | transform 1000 view elements |
40 ms | 46.8 ms | -14.66% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing MoonfaceX:p/xiamengfei.moonface/a2ui-slider (a105197) with main (4e7985e)
Footnotes
-
26 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
React External#1558 Bundle Size — 695.64KiB (0%).a105197(current) vs 4e7985e main#1551(baseline) Bundle metrics
|
| Current #1558 |
Baseline #1551 |
|
|---|---|---|
 Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
17 |
17 |
| Duplicate Modules |
5 |
5 |
| Duplicate Code |
8.59% |
8.59% |
| Packages |
0 |
0 |
| Duplicate Packages |
0 |
0 |
Bundle analysis report Branch MoonfaceX:p/xiamengfei.moonface/... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example with Element Template#712 Bundle Size — 200.08KiB (0%).a105197(current) vs 4e7985e main#705(baseline) Bundle metrics
|
| Current #712 |
Baseline #705 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
 Modules |
90(-2.17%) |
92 |
| Duplicate Modules |
27 |
27 |
| Duplicate Code |
39.8%(+0.1%) |
39.76% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #712 |
Baseline #705 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
54.32KiB |
54.32KiB |
Bundle analysis report Branch MoonfaceX:p/xiamengfei.moonface/... Project dashboard
Generated by RelativeCI Documentation Report issue
React Example#8443 Bundle Size — 237.24KiB (0%).a105197(current) vs 4e7985e main#8436(baseline) Bundle metrics
|
| Current #8443 |
Baseline #8436 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
4 |
4 |
| Modules |
198 |
198 |
| Duplicate Modules |
80 |
80 |
| Duplicate Code |
44.74% |
44.74% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #8443 |
Baseline #8436 |
|
|---|---|---|
| IMG |
145.76KiB |
145.76KiB |
| Other |
91.48KiB |
91.48KiB |
Bundle analysis report Branch MoonfaceX:p/xiamengfei.moonface/... Project dashboard
Generated by RelativeCI Documentation Report issue
React MTF Example#1576 Bundle Size — 208.18KiB (0%).a105197(current) vs 4e7985e main#1569(baseline) Bundle metrics
|
| Current #1576 |
Baseline #1569 |
|
|---|---|---|
| Initial JS |
0B |
0B |
| Initial CSS |
0B |
0B |
| Cache Invalidation |
0% |
0% |
| Chunks |
0 |
0 |
| Assets |
3 |
3 |
| Modules |
193 |
193 |
| Duplicate Modules |
77 |
77 |
| Duplicate Code |
44.24% |
44.24% |
| Packages |
2 |
2 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #1576 |
Baseline #1569 |
|
|---|---|---|
| IMG |
111.23KiB |
111.23KiB |
| Other |
96.95KiB |
96.95KiB |
Bundle analysis report Branch MoonfaceX:p/xiamengfei.moonface/... Project dashboard
Generated by RelativeCI Documentation Report issue
Web Explorer#10017 Bundle Size — 903.49KiB (0%).a105197(current) vs 4e7985e main#10010(baseline) Bundle metrics
|
| Current #10017 |
Baseline #10010 |
|
|---|---|---|
| Initial JS |
45.06KiB |
45.06KiB |
| Initial CSS |
2.22KiB |
2.22KiB |
| Cache Invalidation |
0% |
0% |
| Chunks |
9 |
9 |
| Assets |
11 |
11 |
| Modules |
231 |
231 |
| Duplicate Modules |
11 |
11 |
| Duplicate Code |
27.12% |
27.12% |
| Packages |
10 |
10 |
| Duplicate Packages |
0 |
0 |
Bundle size by type no changes
| Current #10017 |
Baseline #10010 |
|
|---|---|---|
| JS |
499.11KiB |
499.11KiB |
| Other |
402.16KiB |
402.16KiB |
| CSS |
2.22KiB |
2.22KiB |
Bundle analysis report Branch MoonfaceX:p/xiamengfei.moonface/... Project dashboard
Generated by RelativeCI Documentation Report issue
MoonfaceX
force-pushed
the
p/xiamengfei.moonface/a2ui-slider
branch
from
27b450e to
a105197
Compare
2 participants
Summary by CodeRabbit
New Features
Documentation
Style
Tests
Chores
Checklist