Skip to content

Commit 2e08541

Browse files
authored
Fix occluded-content height inflation from inherited line-height (#491)
When occluded-content is rendered inside a table (display: table-row), the debug text ("And X items before/after") can create a line box that inflates the element's actual height above its inline style height. This causes VerticalCollection to measure the wrong height and adjust its estimates, leading to visual jumps when scrolling. Add font-size: 0 and line-height: 0 to prevent the text from affecting layout while keeping it available for screen readers.
1 parent 5728a45 commit 2e08541

File tree

2 files changed

+144
-0
lines changed

2 files changed

+144
-0
lines changed

vertical-collection/src/occluded-content.css

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,16 @@
88

99
/* hides text visually while still being readable by screen readers */
1010
color: rgb(0 0 0 / 0%);
11+
12+
/*
13+
* Prevents the debug text ("And X items before/after") from affecting
14+
* the element's layout height. Without this, inherited line-height values
15+
* can cause the text to create a line box that inflates the element's
16+
* actual height above its inline style height, especially when used
17+
* inside tables with display: table-row.
18+
*/
19+
font-size: 0;
20+
line-height: 0;
1121
}
1222

1323
table .occluded-content,
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import { module, test } from 'qunit';
2+
import { setupRenderingTest } from '../helpers';
3+
import { find, findAll, render } from '@ember/test-helpers';
4+
import { hbs } from 'ember-cli-htmlbars';
5+
import scrollTo from '../helpers/scroll-to';
6+
7+
import getNumbers from 'dummy/lib/get-numbers';
8+
9+
module(
10+
'vertical-collection',
11+
'Integration | Table Occlusion Tests',
12+
function (hooks) {
13+
setupRenderingTest(hooks);
14+
15+
test('occluded-content should have line-height: 0 to prevent text from affecting height', async function (assert) {
16+
// This test verifies that the occluded-content element has line-height: 0
17+
// set in its CSS to prevent the debug text from affecting the element's height.
18+
//
19+
// The issue: When occluded-content is inside a table (display: table-row),
20+
// the debug text ("And X items before/after") creates a line box. If a
21+
// consuming application has a non-zero line-height set, this can cause
22+
// the actual rendered height (getBoundingClientRect().height) to be larger
23+
// than the inline style height. This causes VerticalCollection to "correct"
24+
// its estimate upwards, leading to visual jumps when scrolling.
25+
//
26+
// The fix: The addon's CSS should set `line-height: 0` on `.occluded-content`
27+
// to ensure the debug text never affects the element's layout height.
28+
29+
this.set('items', getNumbers(0, 50));
30+
31+
await render(hbs`
32+
<div class="scrollable" style="height: 200px; overflow-y: scroll;">
33+
<table style="width: 100%;">
34+
<tbody>
35+
<VerticalCollection
36+
@items={{this.items}}
37+
@containerSelector=".scrollable"
38+
@key="number"
39+
@estimateHeight={{40}}
40+
@staticHeight={{true}}
41+
@bufferSize={{0}}
42+
as |item|
43+
>
44+
<tr>
45+
<td style="height: 40px;">{{item.number}}</td>
46+
</tr>
47+
</VerticalCollection>
48+
</tbody>
49+
</table>
50+
</div>
51+
`);
52+
53+
// Scroll down to create occluded content before
54+
await scrollTo('.scrollable', 0, 200);
55+
56+
const occludedBefore = find('.occluded-content:first-of-type');
57+
const occludedAfter = find('.occluded-content:last-of-type');
58+
59+
// The occluded-content elements should exist
60+
assert.ok(occludedBefore, 'occluded-content before exists');
61+
assert.ok(occludedAfter, 'occluded-content after exists');
62+
63+
// Check that line-height is 0 (or "0px" or "normal" which would be equivalent)
64+
const beforeComputedLineHeight =
65+
window.getComputedStyle(occludedBefore).lineHeight;
66+
const afterComputedLineHeight =
67+
window.getComputedStyle(occludedAfter).lineHeight;
68+
69+
// The CSS should explicitly set line-height to 0 to prevent inherited
70+
// line-height from causing the debug text to affect the element's height
71+
assert.strictEqual(
72+
beforeComputedLineHeight,
73+
'0px',
74+
`occluded-content before should have line-height: 0px, got: ${beforeComputedLineHeight}`,
75+
);
76+
77+
assert.strictEqual(
78+
afterComputedLineHeight,
79+
'0px',
80+
`occluded-content after should have line-height: 0px, got: ${afterComputedLineHeight}`,
81+
);
82+
});
83+
84+
test('occluded-content height matches style height in tables', async function (assert) {
85+
// This test verifies that the actual rendered height of occluded-content
86+
// matches its inline style height when used inside tables.
87+
88+
this.set('items', getNumbers(0, 30));
89+
90+
await render(hbs`
91+
<div class="scrollable" style="height: 100px; overflow-y: scroll;">
92+
<table style="width: 100%;">
93+
<tbody>
94+
<VerticalCollection
95+
@items={{this.items}}
96+
@containerSelector=".scrollable"
97+
@key="number"
98+
@estimateHeight={{20}}
99+
@staticHeight={{true}}
100+
@bufferSize={{0}}
101+
as |item|
102+
>
103+
<tr>
104+
<td style="height: 20px;">{{item.number}}</td>
105+
</tr>
106+
</VerticalCollection>
107+
</tbody>
108+
</table>
109+
</div>
110+
`);
111+
112+
// Scroll to ensure we have both occluded content before and after
113+
await scrollTo('.scrollable', 0, 100);
114+
115+
const occludedElements = findAll('.occluded-content');
116+
117+
for (const el of occludedElements) {
118+
const styleHeight = parseFloat(el.style.height);
119+
const actualHeight = el.getBoundingClientRect().height;
120+
const hasText = el.textContent.trim().length > 0;
121+
122+
// Skip elements with 0 height (they have no content to measure)
123+
if (styleHeight === 0) {
124+
continue;
125+
}
126+
127+
assert.ok(
128+
Math.abs(actualHeight - styleHeight) < 1,
129+
`occluded-content (has text: ${hasText}): actual ${actualHeight}px should equal style ${styleHeight}px`,
130+
);
131+
}
132+
});
133+
},
134+
);

0 commit comments

Comments
 (0)