-
Notifications
You must be signed in to change notification settings - Fork 111
fix(web): when estimated-main-axis-size-px is not set, should use the… #1930
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.
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
Conversation
🦋 Changeset detectedLatest commit: 3b331e2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
📝 WalkthroughWalkthroughUpdates x-list CSS to use container sizing and adds fallbacks for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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! |
CodSpeed Performance ReportMerging #1930 will degrade performances by 5.97%Comparing Summary
Benchmarks breakdown
Footnotes
|
Web Explorer#6204 Bundle Size — 366.92KiB (+0.04%).3b331e2(current) vs eee83b6 main#6203(baseline) Bundle metrics
Bundle size by type
Bundle analysis report Branch Sherry-hue:fix/estimated-default Project dashboard Generated by RelativeCI Documentation Report issue |
f310410 to
d56db04
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx (2)
4-4: Remove unused import.
useEffectis imported but never used in this component. Consider removing it to keep imports clean.Apply this diff:
-import { root, useEffect, useRef } from '@lynx-js/react'; +import { root, useRef } from '@lynx-js/react';
8-8: Unused ref - verify if intentional.The
refis created but never attached to any element (line 14'sref={ref}attaches it to the list). While the ref is attached, it's never actually used in the component logic.Compare with
basic-element-list-estimated-main-axis-size-px/index.jsxwhich uses the ref to callautoScroll(). If this test is meant to verify static rendering/layout without dynamic scrolling, the ref might be unnecessary.If the ref isn't needed for this test, consider removing it:
function App() { - const ref = useRef(null); - return ( <view class='page'> <list list-type='single' - ref={ref} scroll-orientation='horizontal' >
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
packages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (7)
.changeset/crazy-pianos-admire.md(1 hunks)packages/web-platform/web-elements/src/XList/x-list.css(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
- packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/crazy-pianos-admire.md
🧠 Learnings (13)
📓 Common learnings
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XList/x-list.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsxpackages/web-platform/web-tests/tests/react.spec.ts.changeset/crazy-pianos-admire.md
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/src/XList/x-list.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/XList/x-list.csspackages/web-platform/web-tests/tests/react.spec.ts
📚 Learning: 2025-10-11T06:16:12.517Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.517Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsxpackages/web-platform/web-tests/tests/react.spec.ts
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-08-13T09:20:00.936Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/crazy-pianos-admire.md
🧬 Code graph analysis (1)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx (2)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx (1)
App(7-54)packages/web-platform/web-tests/tests/react/basic-element-list-horizontal-estimated-main-axis-size-px/index.jsx (1)
App(7-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build / Build (Windows)
- GitHub Check: build / Build (Ubuntu)
- GitHub Check: test-rust / Test (Ubuntu)
🔇 Additional comments (7)
.changeset/crazy-pianos-admire.md (1)
1-5: LGTM!The changeset correctly documents this bug fix with a patch version bump for
@lynx-js/web-elements. The description clearly states the fix behavior.packages/web-platform/web-elements/src/XList/x-list.css (3)
9-9: LGTM! Container queries enabled for fallback sizing.Adding
container-type: sizeenables the container query units (100cqh,100cqw) used in the fallback values below. This is the correct approach for making list items use container-relative sizing whenestimated-main-axis-size-pxis not specified.
65-66: LGTM! Vertical list now uses container height as fallback.The changes correctly implement the fix:
- Added
noneto prevent cross-axis intrinsic sizing- Added
100cqhfallback when--estimated-main-axis-size-pxis not set- This makes vertical lists default to using the full container height
68-70: LGTM! Horizontal list correctly uses container width as fallback.The horizontal orientation rule properly mirrors the vertical behavior:
- Uses
100cqw(container query width) as the fallback- Swaps the sizing order to prioritize width for horizontal scrolling
- Maintains consistency with the vertical implementation
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css (1)
1-22: LGTM! Test styles are appropriate.The CSS provides the necessary layout structure for testing horizontal list behavior with borders and background colors for visual verification.
packages/web-platform/web-tests/tests/react.spec.ts (2)
4555-4572: LGTM! Test verifies default vertical list sizing.The test correctly validates that when
estimated-main-axis-size-pxis not set, the list uses the container height (100cqh) as a fallback and remains scrollable. The test captures before/after scroll screenshots for visual regression verification.
4573-4590: LGTM! Test verifies default horizontal list sizing.The test correctly validates the horizontal orientation case, ensuring that when
estimated-main-axis-size-pxis not set, the horizontal list uses the container width (100cqw) as a fallback. The horizontal scroll by 300px verifies the list is properly sized and scrollable.
d56db04 to
ed2d8ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css (1)
1-20: Consider sharing the CSS between test variants.This CSS file is identical to
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css. While the duplication may be intentional for test isolation, consider whether a shared CSS file could reduce maintenance overhead without compromising test independence.packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx (2)
4-4: Remove unuseduseEffectimport.The
useEffecthook is imported but never used in this component.Apply this diff:
-import { root, useEffect, useRef } from '@lynx-js/react'; +import { root, useRef } from '@lynx-js/react';
8-8: Clarify the purpose of the unused ref or remove it.The
refis created and attached to the list element but never accessed. If this ref is needed for future programmatic list manipulation or assertions, consider adding a comment explaining its purpose. Otherwise, remove it to reduce noise.If the ref is not needed, apply this diff:
function App() { - const ref = useRef(null); - return ( <view class='page'> <list list-type='single' - ref={ref} scroll-orientation='horizontal' >Also applies to: 14-14
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
packages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (7)
.changeset/crazy-pianos-admire.md(1 hunks)packages/web-platform/web-elements/src/XList/x-list.css(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- .changeset/crazy-pianos-admire.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web-platform/web-elements/src/XList/x-list.css
- packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
- packages/web-platform/web-tests/tests/react.spec.ts
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsxpackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsxpackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css
📚 Learning: 2025-11-11T08:05:14.138Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.138Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-10-11T06:16:12.517Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.517Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-08-13T09:20:00.936Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
📚 Learning: 2025-07-16T06:26:22.230Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Vitest (Ubuntu) / check
- GitHub Check: Playwright ALL_ON_UI-SSR (4/4) / check
- GitHub Check: Vitest (Windows) / check
- GitHub Check: Playwright MULTI_THREAD-CSR (4/4) / check
- GitHub Check: Playwright ALL_ON_UI-CSR (3/4) / check
- GitHub Check: benchmark / nodejs-benchmark
- GitHub Check: lighthouse / check
🔇 Additional comments (2)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css (1)
1-20: LGTM!The CSS structure is clean and appropriate for the horizontal list test fixture. The fixed height (500px) correctly constrains the cross-axis for horizontal scrolling, and the color scheme using
--item-indexprovides good visual differentiation for testing.packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx (1)
7-34: Test structure looks good.The component correctly sets up a horizontal list test fixture with 6 items, each properly differentiated by
item-key,id, and--item-indexcustom property. This should effectively test the defaultestimated-main-axis-size-pxbehavior for horizontal scrolling.
ed2d8ba to
3b331e2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx (1)
4-4: Remove unused import.The
useEffectimport is not used in this test fixture.Apply this diff to remove the unused import:
-import { root, useEffect, useRef } from '@lynx-js/react'; +import { root, useRef } from '@lynx-js/react';
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (12)
packages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/initial-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-default/scroll-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/initial-webkit-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-chromium-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-firefox-linux.pngis excluded by!**/*.pngpackages/web-platform/web-tests/tests/react.spec.ts-snapshots/list/basic-element-list-estimated-main-axis-size-px-horizontal-default/scroll-webkit-linux.pngis excluded by!**/*.png
📒 Files selected for processing (7)
.changeset/crazy-pianos-admire.md(1 hunks)packages/web-platform/web-elements/src/XList/x-list.css(2 hunks)packages/web-platform/web-tests/tests/react.spec.ts(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css(1 hunks)packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.jsx
- packages/web-platform/web-tests/tests/react.spec.ts
- packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.css
🧰 Additional context used
📓 Path-based instructions (1)
.changeset/*.md
📄 CodeRabbit inference engine (AGENTS.md)
For contributions, generate and commit a Changeset describing your changes
Files:
.changeset/crazy-pianos-admire.md
🧠 Learnings (18)
📓 Common learnings
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
📚 Learning: 2025-11-03T08:44:10.706Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-elements/src/XList/ListItemAttributes.ts:24-29
Timestamp: 2025-11-03T08:44:10.706Z
Learning: In packages/web-platform/web-elements/src/XList/ListItemAttributes.ts, the `estimated-main-axis-size-px` attribute handler does not need to validate or guard against invalid/NaN values when parsing - data correctness verification is not required for this attribute.
Applied to files:
packages/web-platform/web-elements/src/XList/x-list.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx.changeset/crazy-pianos-admire.mdpackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css
📚 Learning: 2025-09-18T08:12:56.802Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1770
File: packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts:316-318
Timestamp: 2025-09-18T08:12:56.802Z
Learning: In packages/web-platform/web-mainthread-apis/src/utils/processStyleInfo.ts, the current implementation uses cardStyleElement.textContent += for lazy component styles. While this could theoretically invalidate rule indices by reparsing the stylesheet, Sherry-hue indicated that UIDs don't repeat for the same element, making this approach acceptable for now. A future optimization to use separate style elements per entry was discussed but deferred to a separate PR to keep the current lazy bundle PR focused.
Applied to files:
packages/web-platform/web-elements/src/XList/x-list.csspackages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css
📚 Learning: 2025-11-03T08:47:17.714Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1911
File: packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px/index.jsx:14-24
Timestamp: 2025-11-03T08:47:17.714Z
Learning: In the autoScroll method for XList components in the lynx-family/lynx-stack repository, the `rate` parameter supports plain strings like `'100'` in addition to numbers and strings with unit suffixes (`${number}px`, `${number}rpx`, `${number}ppx`), even though the TypeScript type definition may appear more restrictive.
Applied to files:
packages/web-platform/web-elements/src/XList/x-list.css
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/react/testing-library/src/vitest.config.js` is source code for the testing library that gets exported for users, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-11-11T08:05:14.138Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1932
File: packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx:10-26
Timestamp: 2025-11-11T08:05:14.138Z
Learning: In packages/web-platform/web-tests/tests/react/basic-element-x-input-ng-bindinput/index.jsx, the test intentionally uses selectionStart twice in the result string (instead of selectionStart and selectionEnd) because it prioritizes testing whether x-input-ng works functionally, rather than validating the correctness of selection values.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-08-06T13:28:57.182Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1453
File: vitest.config.ts:49-61
Timestamp: 2025-08-06T13:28:57.182Z
Learning: In the lynx-family/lynx-stack repository, the file `packages/rspeedy/create-rspeedy/template-react-vitest-rltl-js/vitest.config.js` is a template file for scaffolding new Rspeedy projects, not a test configuration that should be included in the main vitest projects array.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-08-13T09:20:00.936Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1502
File: packages/react/testing-library/types/entry.d.ts:71-71
Timestamp: 2025-08-13T09:20:00.936Z
Learning: In lynx-js/react testing library, wrapper components must have children as a required prop because they are always called with `h(WrapperComponent, null, innerElement)` where innerElement is passed as children. The type `React.JSXElementConstructor<{ children: React.ReactNode }>` correctly requires children to be mandatory.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-10-11T06:16:12.517Z
Learnt from: Sherry-hue
Repo: lynx-family/lynx-stack PR: 1820
File: packages/web-platform/web-tests/tests/react.spec.ts:834-856
Timestamp: 2025-10-11T06:16:12.517Z
Learning: In packages/web-platform/web-tests/tests/react.spec.ts, the tests `basic-bindmouse` and `basic-mts-bindtouchstart` are NOT duplicates despite having similar test structures. They test different event types: `basic-bindmouse` validates mouse events (mousedown, mouseup, mousemove) with mouse-specific properties (button, buttons, x, y, pageX, pageY, clientX, clientY), while `basic-mts-bindtouchstart` validates touch events (touchstart) with touch arrays (touches, targetTouches, changedTouches). The similar test structure is coincidental and follows testing conventions.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, private packages (marked with "private": true in package.json) like lynx-js/react-transform don't require meaningful changeset entries even when their public APIs change, since they are not published externally and only affect internal development.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx.changeset/crazy-pianos-admire.md
📚 Learning: 2025-08-11T05:57:18.212Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/testing-library/testing-environment/src/index.ts:255-258
Timestamp: 2025-08-11T05:57:18.212Z
Learning: In the ReactLynx testing environment (`packages/testing-library/testing-environment/src/index.ts`), the dual assignment pattern `target.console.method = console.method = () => {}` is required for rstest compatibility. This is because rstest provides `console` in an IIFE (Immediately Invoked Function Expression), and both the target and global console need to have these methods defined for proper test execution.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-08-11T05:59:28.530Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1305
File: packages/react/testing-library/src/plugins/vitest.ts:4-6
Timestamp: 2025-08-11T05:59:28.530Z
Learning: In the lynx-family/lynx-stack repository, the `packages/react/testing-library` package does not have `vite` as a direct dependency. It relies on `vitest` being available from the monorepo root and accesses Vite types through re-exports from `vitest/node`. Direct imports from `vite` should not be suggested for this package.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-07-16T06:26:22.230Z
Learnt from: PupilTong
Repo: lynx-family/lynx-stack PR: 1029
File: packages/web-platform/web-core-server/src/createLynxView.ts:0-0
Timestamp: 2025-07-16T06:26:22.230Z
Learning: In the lynx-stack SSR implementation, each createLynxView instance is used to render once and then discarded. There's no reuse of the same instance for multiple renders, so event arrays and other state don't need to be cleared between renders.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx
📚 Learning: 2025-09-12T09:43:04.847Z
Learnt from: gaoachao
Repo: lynx-family/lynx-stack PR: 1736
File: .changeset/spotty-experts-smoke.md:1-3
Timestamp: 2025-09-12T09:43:04.847Z
Learning: In the lynx-family/lynx-stack repository, empty changeset files (containing only `---\n\n---`) are used for internal changes that modify src/** files but don't require meaningful release notes, such as private package changes or testing-only modifications. This satisfies CI requirements without generating user-facing release notes.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-07-22T09:23:07.797Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:23:07.797Z
Learning: In the lynx-family/lynx-stack repository, changesets are only required for meaningful changes to end-users such as bugfixes and features. Internal/development changes like chores, refactoring, or removing debug info do not need changeset entries.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-07-22T09:26:16.722Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1330
File: .changeset/olive-animals-attend.md:1-3
Timestamp: 2025-07-22T09:26:16.722Z
Learning: In the lynx-family/lynx-stack repository, CI checks require changesets when files matching the pattern "src/**" are modified (as configured in .changeset/config.json). For internal changes that don't need meaningful changesets, an empty changeset file is used to satisfy the CI requirement while not generating any release notes.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-08-07T04:00:59.645Z
Learnt from: colinaaa
Repo: lynx-family/lynx-stack PR: 1454
File: pnpm-workspace.yaml:46-46
Timestamp: 2025-08-07T04:00:59.645Z
Learning: In the lynx-family/lynx-stack repository, the webpack patch (patches/webpack5.101.0.patch) was created to fix issues with webpack5.99.9 but only takes effect on webpack5.100.0 and later versions. The patchedDependencies entry should use "webpack@^5.100.0" to ensure the patch applies to the correct version range.
Applied to files:
.changeset/crazy-pianos-admire.md
📚 Learning: 2025-09-23T08:54:39.966Z
Learnt from: upupming
Repo: lynx-family/lynx-stack PR: 1670
File: packages/webpack/css-extract-webpack-plugin/test/hotCases/hot/hot-update-json/dual-thread/__snapshot__/index.css:6-8
Timestamp: 2025-09-23T08:54:39.966Z
Learning: In the lynx-stack CSS extract webpack plugin tests, many test fixture CSS files intentionally use invalid CSS syntax like `color: 'red';` with quoted values. The snapshots correctly reflect this invalid CSS from the source fixtures. To fix CSS validation issues, the source fixture files should be updated first, then snapshots regenerated, rather than manually editing snapshots.
Applied to files:
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css
🔇 Additional comments (4)
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-horizontal-default/index.css (1)
1-20: LGTM! Test fixture CSS is well-structured.The CSS properly defines layout styles for the horizontal list test, with appropriate element selectors and dynamic styling using CSS custom properties.
packages/web-platform/web-tests/tests/react/basic-element-list-estimated-main-axis-size-px-default/index.jsx (1)
7-31: LGTM! Test component is correctly structured.The component properly creates a ref, attaches it to the list element, and renders six list-items with appropriate keys, IDs, and custom properties for testing.
packages/web-platform/web-elements/src/XList/x-list.css (1)
9-9: Excellent fix! Container query fallback correctly implements the PR objective.The changes properly address the issue:
Line 9:
container-type: sizeenables container queries, making the x-list itself the sizing reference for child elements.Line 65: For vertical lists,
none auto var(--estimated-main-axis-size-px, 100cqh)separates width (none = use actual) from height (auto with fallback to 100% of container height).Lines 68-70: For horizontal lists,
auto var(--estimated-main-axis-size-px, 100cqw) noneestimates width (main axis) and uses actual height (cross axis).When
--estimated-main-axis-size-pxis not set, list-items now correctly fall back to the x-list container's height (vertical) or width (horizontal), exactly as described in the PR objectives.Also applies to: 65-70
.changeset/crazy-pianos-admire.md (1)
1-5: LGTM! Changeset appropriately documents the bug fix.The changeset correctly identifies this as a patch release for
@lynx-js/web-elementswith a clear description of the fix. The description captures the essence of the change, which applies to both vertical (height) and horizontal (width) orientations.
React Example#6169 Bundle Size — 234.35KiB (0%).3b331e2(current) vs eee83b6 main#6168(baseline) Bundle metrics
|
| Current #6169 |
Baseline #6168 |
|
|---|---|---|
0B |
0B |
|
0B |
0B |
|
0% |
0% |
|
0 |
0 |
|
4 |
4 |
|
164 |
164 |
|
66 |
66 |
|
46.71% |
46.71% |
|
2 |
2 |
|
0 |
0 |
Bundle size by type no changes
| Current #6169 |
Baseline #6168 |
|
|---|---|---|
145.76KiB |
145.76KiB |
|
88.59KiB |
88.59KiB |
Bundle analysis report Branch Sherry-hue:fix/estimated-default Project dashboard
Generated by RelativeCI Documentation Report issue
… height of list
Summary by CodeRabbit
Bug Fixes
Style
Tests
Chores
Checklist