Skip to content

test(ui): count .conv-stream padding in the scroll test's scrollHeight - #2449

Merged
mattwilkinsonn merged 1 commit into
mainfrom
compass-native/rig-5818-scroll-stub-padding
Oct 19, 2026
Merged

mattwilkinsonn merged 1 commit into
mainfrom
compass-native/rig-5818-scroll-stub-padding

Conversation

@rigel-mintaka

@rigel-mintaka rigel-mintaka commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

The scroll test's geometry stub now counts .conv-stream's vertical padding in scrollHeight (RIG-5818).

A browser's scrollHeight includes the scroller's padding. .conv-stream has 12px top and bottom (apps/ui/src/app.css). The stub returned only the sizer height, so its max scroll was 24px short. The scrollTop clamp from #2444 therefore stopped end-scroll writes early. The stub now adds the padding before flooring at clientHeight.

Test-only change, based on main (the virtual-core stack #2411, #2444, #2445 has merged).

Verification

  • bun test --conditions browser src/components/ in apps/ui: 495 pass, 0 fail.
  • moon run compass-ui:ci passes. biome is clean.

Risks

None at runtime. No current test asserts the exact bottom offset, so this corrects the fixture rather than turning a test red or green.

Compatibility

No change.

Documentation

None.

Ledger-impact: none (test fixture only)

@trunk-io

trunk-io Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

😎 This pull request was merged.

@linear-code

linear-code Bot commented Oct 18, 2026

Copy link
Copy Markdown

RIG-5818

@rigel-mintaka
rigel-mintaka force-pushed the compass-native/rig-5818-scroll-stub-padding branch from 74db4a6 to da29e34 Compare October 18, 2026 21:40
@rigel-mintaka rigel-mintaka changed the title chore(deps): update typescript dependencies test(ui): count .conv-stream padding in the scroll test's scrollHeight Oct 18, 2026
@rigel-mintaka

Copy link
Copy Markdown
Contributor Author

Review at head da29e34 (rebased onto main 23b79aa after the virtual-core stack merged): high 0, medium 0, low 2. Low 1 (setup comment still said scrollHeight = content height) is fixed in this head. Low 2 suggested asserting the stub's own scrollHeight. Not taken: that would test the fixture against itself, not any component behaviour. The reviewer confirmed the padding model (scrollHeight includes padding; clientHeight already does). Local: ChannelView tests 495/495, biome clean.

@github-actions

github-actions Bot commented Oct 18, 2026 •

Copy link
Copy Markdown

Compass engineering docs preview: https://compass-native-rig-5818-scro.compass-eng-docs.pages.dev

Deployed from compass-native/rig-5818-scroll-stub-padding at 518092e.

@rigel-mintaka
rigel-mintaka marked this pull request as ready for review October 18, 2026 21:49
@rigel-mintaka
rigel-mintaka force-pushed the compass-native/rig-5818-scroll-stub-padding branch from da29e34 to 518092e Compare October 18, 2026 23:51
@rigel-mintaka

Copy link
Copy Markdown
Contributor Author

Review at head 518092e (rebased onto main after #2433's comment trims; took main's file and re-applied only the padding change, 7+/3-): high 0, medium 0, low 0 open. The earlier stale-comment low is gone, because #2433 removed that comment. Local: ChannelView tests 495/495, compass-ui:ci pass, biome clean.

@mattwilkinsonn
mattwilkinsonn merged commit 9a4ab4f into main Oct 19, 2026
20 checks passed
@mattwilkinsonn
mattwilkinsonn deleted the compass-native/rig-5818-scroll-stub-padding branch October 19, 2026 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants