-
Notifications
You must be signed in to change notification settings - Fork 99
Track Responsive-Design carousel resize events #579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -163,6 +163,46 @@ class PageElement { | |
| this.#node.scrollIntoView(options); | ||
| } | ||
|
|
||
| observeResizeEvents() { | ||
| const contentWindow = this.#node.ownerDocument.defaultView; | ||
| const state = { | ||
| count: 0, | ||
| lastWidth: null, | ||
| }; | ||
| let markReady; | ||
| // Resolves on the first delivery so callers can seed a baseline before resizing. | ||
| const ready = new Promise((resolve) => { | ||
| markReady = resolve; | ||
| }); | ||
| const observer = new contentWindow.ResizeObserver((entries) => { | ||
| for (const entry of entries) { | ||
| const contentBoxSize = entry.contentBoxSize; | ||
| const inlineSize = Array.isArray(contentBoxSize) ? contentBoxSize[0]?.inlineSize : contentBoxSize?.inlineSize; | ||
| const width = inlineSize ?? entry.contentRect.width; | ||
| // The first delivery seeds the baseline width without counting it as a change. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: I was a bit confused about "delivery" :) I "callback" might be more common? |
||
| if (state.lastWidth === null) { | ||
| state.lastWidth = width; | ||
| markReady(); | ||
| continue; | ||
| } | ||
| if (width === state.lastWidth) | ||
| continue; | ||
| state.count++; | ||
| state.lastWidth = width; | ||
| } | ||
| }); | ||
| observer.observe(this.#node); | ||
| return { | ||
| ready, | ||
| get count() { | ||
| return state.count; | ||
| }, | ||
| disconnect() { | ||
| observer.disconnect(); | ||
| }, | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it make sense to have a single stop() api here that disconnects and returns the count? |
||
| }; | ||
| } | ||
|
|
||
| dispatchEvent(eventName, options = NATIVE_OPTIONS, eventType = Event) { | ||
| if (eventName === "submit") | ||
| // FIXME FireFox doesn't like `new Event('submit') | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -214,6 +214,9 @@ export const ExperimentalSuites = freezeSuites([ | |
| new BenchmarkTestStep("ReduceWidthIn5Steps", async (page) => { | ||
| const widths = [768, 704, 640, 560, 480]; | ||
| const MATCH_MEDIA_QUERY_BREAKPOINT = 640; | ||
| const carouselResizeObservations = page.querySelector(".carousel", ["cooking-app", "main-content", "recipe-carousel"]).observeResizeEvents(); | ||
| // Seed the baseline width before the synchronous width changes below. | ||
| await carouselResizeObservations.ready; | ||
|
|
||
| // The matchMedia query is "(max-width: 640px)" | ||
| // Starting from a width > 640px, we'll only get 1 event when crossing to <= 640px | ||
|
|
@@ -230,6 +233,12 @@ export const ExperimentalSuites = freezeSuites([ | |
| } | ||
|
|
||
| await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); | ||
| const { count } = carouselResizeObservations; | ||
| carouselResizeObservations.disconnect(); | ||
| if (count) | ||
| console.warn(`ReduceWidthIn5Steps: recipe-carousel ResizeObserver delivered ${count} coalesced width change(s).`); | ||
| else | ||
| console.warn("ReduceWidthIn5Steps: recipe-carousel ResizeObserver delivered 0 width changes; expected width changes during iframe resize."); | ||
| }), | ||
| new BenchmarkTestStep("ScrollToChatAndSendMessages", async (page) => { | ||
| const cvWorkComplete = new Promise((resolve) => { | ||
|
|
@@ -273,6 +282,9 @@ export const ExperimentalSuites = freezeSuites([ | |
| new BenchmarkTestStep("IncreaseWidthIn5Steps", async (page) => { | ||
| const widths = [560, 640, 704, 768, 800]; | ||
| const MATCH_MEDIA_QUERY_BREAKPOINT = 704; | ||
| const carouselResizeObservations = page.querySelector(".carousel", ["cooking-app", "main-content", "recipe-carousel"]).observeResizeEvents(); | ||
| // Seed the baseline width before the synchronous width changes below. | ||
| await carouselResizeObservations.ready; | ||
|
|
||
| // The matchMedia query is "(max-width: 640px)" | ||
| // Starting from a width <= 640px, we'll get 1 event when crossing back to > 640px. | ||
|
|
@@ -289,6 +301,12 @@ export const ExperimentalSuites = freezeSuites([ | |
| } | ||
|
|
||
| await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); | ||
| const { count } = carouselResizeObservations; | ||
| carouselResizeObservations.disconnect(); | ||
| if (count) | ||
| console.warn(`IncreaseWidthIn5Steps: recipe-carousel ResizeObserver delivered ${count} coalesced width change(s).`); | ||
| else | ||
| console.warn("IncreaseWidthIn5Steps: recipe-carousel ResizeObserver delivered 0 width changes; expected width changes during iframe resize."); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: maybe add a helper to print this message? |
||
| }), | ||
| ], | ||
| }, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specific use-case you had in mind for detaching observer setup and sarting it?
I think we could just either: