sadpandajoe commented on PR #35806:
URL: https://github.com/apache/superset/pull/35806#issuecomment-3459667684
From codex, we should see if the test will catch the regression or not.
> The configured SCREENSHOT_TILED_VIEWPORT_HEIGHT (default 2000) onto the
scroll math, while Playwright’s browser window stays around 1200px tall. With
your new clipping logic, Playwright no longer throws, but we quietly skip
content after the first tile (0–1200, then we jump to 2000–3200, leaving ~800px
unaccounted for). To fix both the crash and the coverage, we can do the
following:
>
> - Fetch the actual viewport height once (actual_height =
page.viewport_size["height"] or via window.innerHeight) and set tile_height =
min(viewport_height, actual_height).
> - Use tile_height for both num_tiles and the scroll increment instead of
the configured viewport_height.
> - Keep the original clip dictionary (with a quick clip_y = max(0,
clip_y) guard if needed), so we still capture the full element per tile.
>
> Here’s a regression test you can drop under TestTakeTiledScreenshot to
lock the behavior down:
```
def test_scroll_increment_respects_actual_viewport_height(self):
"""When config viewport height > actual viewport, we still cover
every tile."""
mock_page = MagicMock()
mock_page.viewport_size = {"width": 1600, "height": 1200}
element = MagicMock()
mock_page.locator.return_value = element
element_info = {"height": 3600, "top": 0, "left": 0, "width": 800}
element_box = {"x": 0, "y": 0, "width": 800, "height": 1200}
mock_page.evaluate.side_effect = [
element_info,
None, element_box, # first tile
None, element_box, # second tile
None, element_box, # third tile
None, # reset scroll
]
mock_page.screenshot.return_value = b"screenshot"
with
patch("superset.utils.screenshot_utils.combine_screenshot_tiles"):
take_tiled_screenshot(mock_page, "dashboard",
viewport_height=2000)
# We expect three tiles (0–1200, 1200–2400, 2400–3600) even though
config says 2000.
assert mock_page.screenshot.call_count == 3
scroll_calls = [
call.args[0]
for call in mock_page.evaluate.call_args_list
if call.args[0].startswith("window.scrollTo")
]
assert scroll_calls == [
"window.scrollTo(0, 0)",
"window.scrollTo(0, 1200)",
"window.scrollTo(0, 2400)",
]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
