Skip to content

fix: getVisibleCells return empty arr of getVisibleCells#2308

Merged
Sherry-hue merged 1 commit intolynx-family:mainfrom
Sherry-hue:fix/list-getVisibleCells
Mar 5, 2026
Merged

fix: getVisibleCells return empty arr of getVisibleCells#2308
Sherry-hue merged 1 commit intolynx-family:mainfrom
Sherry-hue:fix/list-getVisibleCells

Conversation

@Sherry-hue
Copy link
Copy Markdown
Collaborator

@Sherry-hue Sherry-hue commented Mar 5, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Fixed an issue where visible cell detection functionality was not working correctly in Firefox. The implementation now includes a geometry-based workaround that ensures accurate visibility calculations across all browsers.
  • Tests

    • Updated test suite to execute visibility tests on all browsers instead of skipping specific browser engines, and strengthened validation checks to ensure correct data is returned.

Checklist

  • Tests updated (or not required).
  • Documentation updated (or not required).
  • Changeset added, and when a BREAKING CHANGE occurs, it needs to be clearly marked (or not required).

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 5, 2026

🦋 Changeset detected

Latest commit: a3bf01a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@lynx-js/web-elements Patch
@lynx-js/web-core-wasm Patch
@lynx-js/template-webpack-plugin Patch
@lynx-js/react-rsbuild-plugin Patch
@lynx-js/react-alias-rsbuild-plugin Patch

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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 5, 2026

📝 Walkthrough

Walkthrough

This PR introduces a fix for the getVisibleCells method in the XList component to address a Firefox compatibility issue where contentvisibilityautostatechange doesn't propagate to list-item elements. The fix adds a geometry-based fallback to recompute visible cells, updates the test suite to run on all browsers, and creates a changelog entry documenting the patch.

Changes

Cohort / File(s) Summary
Changelog Entry
.changeset/social-jobs-behave.md
New patch release note documenting the fix for getVisibleCells Firefox compatibility issue with contentvisibilityautostatechange not propagating to list-item elements.
Core Fix
packages/web-platform/web-elements/src/elements/XList/XList.ts
Modified getVisibleCells to add a geometry-based fallback; when the visibility map yields no cells, the method now filters LIST-ITEM children by intersection with the list container's bounding rect to ensure correct visibility calculation in Firefox.
Test Updates
packages/web-platform/web-elements/tests/web-elements.spec.ts
Removed Firefox/WebKit skip condition from get-visible-cells test to run on all browsers; tightened assertion to require non-empty array (data.length > 0) in addition to array type validation.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A Firefox friend needed some care,
When cells hid beyond compare,
So we mapped the geometry,
Added a fallback decree,
Now visibility blooms everywhere! 🐰✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title mentions 'getVisibleCells return empty arr' which is grammatically awkward and repetitive, but it does reference the core fix: handling empty array returns in getVisibleCells. However, it fails to capture the main technical issue (Firefox compatibility with contentvisibilityautostatechange) that the changes actually address. Consider revising the title to better reflect the root cause, such as: 'fix: getVisibleCells not working in Firefox due to contentvisibilityautostatechange not propagating' or 'fix: getVisibleCells fallback for Firefox when contentvisibilityautostatechange unavailable'.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Tip

Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).
Share your feedback on Discord.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 5, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.changeset/social-jobs-behave.md:
- Line 5: Rewrite the changelog line to a clear, user-facing sentence: replace
the current text "fix: getVisibleCells cannot work in firefox due to
contentvisibilityautostatechange not propagate list-item" with a polished
version such as "fix: getVisibleCells failed in Firefox because
ContentVisibilityAutoStateChange events did not propagate for list-item
elements" — ensure you keep the references to getVisibleCells, Firefox,
ContentVisibilityAutoStateChange, and list-item so the cause and scope remain
clear.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b6d817bd-e377-4cfa-8826-54d66f60a820

📥 Commits

Reviewing files that changed from the base of the PR and between 5fc68ae and a3bf01a.

📒 Files selected for processing (3)
  • .changeset/social-jobs-behave.md
  • packages/web-platform/web-elements/src/elements/XList/XList.ts
  • packages/web-platform/web-elements/tests/web-elements.spec.ts

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Mar 5, 2026

Merging this PR will degrade performance by 8.27%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 70 untouched benchmarks
⏩ 3 skipped benchmarks1

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
basic-performance-small-css 8.3 ms 7.7 ms +7.88%
basic-performance-nest-level-100 7 ms 7.7 ms -8.27%

Comparing Sherry-hue:fix/list-getVisibleCells (a3bf01a) with main (5fc68ae)

Open in CodSpeed

Footnotes

  1. 3 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.

@relativeci
Copy link
Copy Markdown

relativeci bot commented Mar 5, 2026

Web Explorer

#7990 Bundle Size — 384.21KiB (+0.05%).

a3bf01a(current) vs 5fc68ae main#7973(baseline)

Bundle metrics  Change 3 changes Regression 1 regression
                 Current
#7990
     Baseline
#7973
Regression  Initial JS 155.31KiB(+0.11%) 155.13KiB
No change  Initial CSS 35.1KiB 35.1KiB
Change  Cache Invalidation 40.39% 56.15%
No change  Chunks 8 8
No change  Assets 8 8
Change  Modules 237(-0.84%) 239
No change  Duplicate Modules 16 16
No change  Duplicate Code 2.98% 2.98%
No change  Packages 4 4
No change  Duplicate Packages 0 0
Bundle size by type  Change 1 change Regression 1 regression
                 Current
#7990
     Baseline
#7973
Regression  JS 253.26KiB (+0.07%) 253.09KiB
No change  Other 95.85KiB 95.85KiB
No change  CSS 35.1KiB 35.1KiB

Bundle analysis reportBranch Sherry-hue:fix/list-getVisibleCe...Project dashboard


Generated by RelativeCIDocumentationReport issue

@Sherry-hue Sherry-hue enabled auto-merge (squash) March 5, 2026 11:58
@Sherry-hue Sherry-hue merged commit 7518b72 into lynx-family:main Mar 5, 2026
78 of 81 checks passed
colinaaa pushed a commit that referenced this pull request Mar 9, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## @lynx-js/devtool-mcp-server@0.5.0

### Minor Changes

- Use `@lynx-js/devtool-connector` instead of
`@lynx-js/debug-router-connector`.
([#2284](#2284))

The new connector avoids using keep-alive connections, which makes the
connection much more reliable.

- **BREAKING CHANGE**: Remove the `./debug-router-connector` exports.
([#2284](#2284))

## @lynx-js/web-elements@0.12.0

### Minor Changes

- feat: add `willchange` event to `x-viewpager-ng`
([#2305](#2305))

### Patch Changes

- fix: firefox `@supports(width:1rex)`
([#2288](#2288))

- fix: check computed overflow style in `getTheMostScrollableKid` to
avoid treating `overflow: visible` elements as scroll containers
([#2309](#2309))

- fix: the inline-truncation should only work as a direct child of
x-text ([#2287](#2287))

- fix: getVisibleCells cannot work in firefox due to
contentvisibilityautostatechange not propagate list-item
([#2308](#2308))

- fix: foldview stuck issue
([#2304](#2304))

## @lynx-js/gesture-runtime@2.1.3

### Patch Changes

- Optimize gesture callbacks and relationships to prevent unnecessary
gesture registration and rerenders.
([#2277](#2277))

## @lynx-js/react@0.116.5

### Patch Changes

- Improve React runtime hook profiling.
([#2235](#2235))
Enable Profiling recording first, then enter the target page so the
trace includes full render/hydrate phases.

- Record trace events for `useEffect` / `useLayoutEffect` hook entry,
callback, and cleanup phases.
    -   Log trace events for `useState` setter calls.
- Wire `profileFlowId` support in debug profile utilities and attach
flow IDs to related hook traces.
- Instrument hydrate/background snapshot profiling around patch
operations with richer args (e.g. snapshot id/type, dynamic part index,
value type, and source when available).
- Capture vnode source mapping in dev and use it in profiling args to
improve trace attribution.
- Expand debug test coverage for profile utilities, hook profiling
behavior, vnode source mapping, and hydrate profiling branches.

- refactor: call loadWorkletRuntime once in each module
([#2315](#2315))

## @lynx-js/rspeedy@0.13.5

### Patch Changes

- feat: opt-in the web platform's new binary output format
([#2281](#2281))

    Introduce a new flag to enable the new binary output format.

Currently it's an internal-use-only flag that will be removed in the
future; set the corresponding environment variable to 'true' to enable
it.

- Avoid generating `Rsbuild vundefined` in greeting message.
([#2275](#2275))

-   Updated dependencies \[]:
    -   @lynx-js/web-rsbuild-server-middleware@0.19.8

## @lynx-js/lynx-bundle-rslib-config@0.2.2

### Patch Changes

- Support bundle and load css in external bundle
([#2143](#2143))

## @lynx-js/external-bundle-rsbuild-plugin@0.0.3

### Patch Changes

- Updated dependencies
\[[`c28b051`](c28b051),
[`4cbf809`](4cbf809)]:
    -   @lynx-js/externals-loading-webpack-plugin@0.0.4

## @lynx-js/react-rsbuild-plugin@0.12.10

### Patch Changes

- Support bundle and load css in external bundle
([#2143](#2143))

- Updated dependencies
\[[`59f2933`](59f2933),
[`453e006`](453e006)]:
    -   @lynx-js/template-webpack-plugin@0.10.5
    -   @lynx-js/css-extract-webpack-plugin@0.7.0
    -   @lynx-js/react-webpack-plugin@0.7.4
    -   @lynx-js/react-alias-rsbuild-plugin@0.12.10
    -   @lynx-js/use-sync-external-store@1.5.0
    -   @lynx-js/react-refresh-webpack-plugin@0.3.4

## @lynx-js/web-core-wasm@0.0.5

### Patch Changes

- Updated dependencies
\[[`4963907`](4963907),
[`8fd936a`](8fd936a),
[`0d41253`](0d41253),
[`d32c4c6`](d32c4c6),
[`7518b72`](7518b72),
[`fca9d4a`](fca9d4a)]:
    -   @lynx-js/web-elements@0.12.0

## @lynx-js/externals-loading-webpack-plugin@0.0.4

### Patch Changes

- perf: optimize external bundle loading by merging multiple
`fetchBundle` calls for the same URL into a single request.
([#2307](#2307))

- Support bundle and load css in external bundle
([#2143](#2143))

## @lynx-js/template-webpack-plugin@0.10.5

### Patch Changes

- feat: allow `templateDebugUrl` to be customized via
`output.publicPath` or the `beforeEncode` hook.
([#2274](#2274))

- feat: opt-in the web platform's new binary output format
([#2281](#2281))

    Introduce a new flag to enable the new binary output format.

Currently it's an internal-use-only flag that will be removed in the
future; set the corresponding environment variable to 'true' to enable
it.

-   Updated dependencies \[]:
    -   @lynx-js/web-core-wasm@0.0.5

## create-rspeedy@0.13.5



## @lynx-js/react-alias-rsbuild-plugin@0.12.10



## upgrade-rspeedy@0.13.5

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants