Scroll on 'main scrollable element' when possible #405
Conversation
|
seanmcguire12
added
act
|
|
||
| await stagehand.close(); | ||
| const listings = real_estate_listings.listings; | ||
| const expectedLength = 38; |
There was a problem hiding this comment.
are we sure this is fixed? this doesn't seem sustainable
| @@ -19,7 +19,10 @@ declare global { | |||
| }>; | |||
| debugDom: () => Promise<void>; | |||
| cleanupDebug: () => void; | |||
| scrollToHeight: (height: number) => Promise<void>; | |||
| scrollToHeight: ( | |||
There was a problem hiding this comment.
nit: can we make this serialized json
| let mainScrollable: HTMLElement = document.documentElement; | ||
| let maxScrollableArea = 0; | ||
|
|
||
| // Find the largest candidate |
There was a problem hiding this comment.
so this is basically just getting literally the biggest element and marking that main? using this approach should totally be opt-in; i feel like this can get funky super fast
| // Verify we can actually scroll that candidate | ||
| if (mainScrollable !== document.documentElement) { | ||
| if (!canElementScroll(mainScrollable)) { | ||
| console.log( |
There was a problem hiding this comment.
future note: we should consolidate DOM logs to a window.log function
| export async function scrollToHeight(height: number) { | ||
| window.scrollTo({ top: height, left: 0, behavior: "smooth" }); | ||
| export async function scrollToHeight( | ||
| containerOrWindow: HTMLElement | Window, |
There was a problem hiding this comment.
let's name this differently and make it a custom type as well -- something like mainContainer: StagehandContainer, where StagehandContainer is HTMLElement | Window.
that way you can just do something like mainContainer.scrollTo({ top: height, left: 0, behavior: "smooth" }) without all these if else statements
| clearTimeout(scrollEndTimer); | ||
| scrollEndTimer = window.setTimeout(() => { | ||
| window.removeEventListener("scroll", handleScrollEnd); | ||
| if (containerOrWindow instanceof Window) { |
There was a problem hiding this comment.
should not be an if statement; this should be one method
| window.addEventListener("scroll", handleScrollEnd, { passive: true }); | ||
| handleScrollEnd(); | ||
| if (containerOrWindow instanceof Window) { | ||
| containerOrWindow.addEventListener("scroll", handleScroll, { |
There was a problem hiding this comment.
should also not be in an if statement
| let viewportHeight: number; | ||
| let totalScrollHeight: number; | ||
|
|
||
| if (container instanceof Window) { |
why
examples
Zillow:
apartments.com
what changed
processAllOfDomto attempt scrolling on a large scrollable container, and fall back to the original approach (scrolling on the root DOM) if there are no large scrollable containersimportant note
processAllOfDomprocessDomto enableactto take actions on elements that are positioned deep within a scrollable containertest plan