Experimental Workload: Responsive Design #453

issackjohn · 2024-11-15T17:46:24Z

This pull request introduces a new experimental responsive design workload, featuring a single-page cooking website built with Lit and Tailwind CSS.

The hosted version can be found here.
Quick link to dev mode with workload selected.

Please look at the README in experimental/responsive-design for more information about the code and how it is organized.

Not included in this PR:

Support for different languages

issackjohn · 2024-11-15T19:29:31Z

Adding reviewers for visibility, feel free to take a look :)

rniwa · 2024-11-15T19:47:15Z

It seems to me that the test doesn't really take the advantage of responsive design since iframe's size is always same. Maybe we should add steps to resize iframe so that responsive design comes into play?

issackjohn · 2024-11-15T19:57:48Z

It seems to me that the test doesn't really take the advantage of responsive design since iframe's size is always same. Maybe we should add steps to resize iframe so that responsive design comes into play?

Currently, we are resizing the the iframe with id #content-iframe in the ReduceWidthIn5Steps & IncreaseWidthIn5Steps steps. Do you mean that we should be resizing a different iframe?

rniwa · 2024-11-15T20:14:05Z

It seems to me that the test doesn't really take the advantage of responsive design since iframe's size is always same. Maybe we should add steps to resize iframe so that responsive design comes into play?

Currently, we are resizing the the iframe with id #content-iframe in the ReduceWidthIn5Steps & IncreaseWidthIn5Steps steps. Do you mean that we should be resizing a different iframe

Oh, I see. We're resizing the inner iframe. I guess that works.

issackjohn · 2024-12-02T19:46:15Z

Gentle ping 😄

camillobruni · 2024-12-03T14:35:09Z

I still see rather short run times for individual steps (close to 1.6ms in chrome canary) which might introduce a bit too much noise in the long run.

Responsive-Design
Responsive-Design/LoadChatAndExpandRecipe	5.85	±	0.46 (7.9%)	ms
Responsive-Design/ReduceWidthIn5Steps  12.96	±	0.45 (3.5%)	ms
Responsive-Design/ScrollToChatAndSendMessage	1.68	±	0.07 (4.4%)	ms
Responsive-Design/IncreaseWidthIn5Steps	11.37	±	0.28 (2.5%)

ScrollToChatAndSendMessage and (and especially it's sub-items) will show some aliasing from timer resolution which is ~ 0.1ms on chrome.
In general, i'd bump the step durations by 1.5x or 2x even to be on par with other workloads.

camillobruni · 2024-12-03T14:42:01Z

experimental/responsive-design/index.html

+        <title>Cooking with Lit and Tailwind</title>
+    </head>
+    <body style="overflow: hidden; margin: 0">
+        <iframe title="Cooking Website with Recipes" id="content-iframe" src="iframe.html" width="800px" height="600px" style="border: none"></iframe>


Can we start off with 100% width/height as the maximum size (the step-resize sounds ok to be done with explicit sizes again)?
Albeit not fully exposed, we can set a custom viewport size in the params.mjs and it's used in the runner.
This workload would currently not support this directly.

Sounds good to me

issackjohn · 2024-12-05T23:26:44Z

I still see rather short run times for individual steps (close to 1.6ms in chrome canary) which might introduce a bit too much noise in the long run.
Responsive-Design
Responsive-Design/LoadChatAndExpandRecipe	5.85	±	0.46 (7.9%)	ms
Responsive-Design/ReduceWidthIn5Steps  12.96	±	0.45 (3.5%)	ms
Responsive-Design/ScrollToChatAndSendMessage	1.68	±	0.07 (4.4%)	ms
Responsive-Design/IncreaseWidthIn5Steps	11.37	±	0.28 (2.5%)	
ScrollToChatAndSendMessage and (and especially it's sub-items) will show some aliasing from timer resolution which is ~ 0.1ms on chrome. In general, i'd bump the step durations by 1.5x or 2x even to be on par with other workloads.

To bump up the step durations, I am thinking of

Expanding all the recipes as part of LoadChatAndExpandRecipe
Collapsing all the recipes as part of ScrollToChatAndSendMessage
2.1 Sending a few more messages in the chat instead of just one.

currently no proposed changes for ReduceWidthIn5Steps & IncreaseWidthIn5Steps

flashdesignory · 2024-12-10T03:12:13Z

Some visual questions - these might not be relevant to the tests:

Is this spacing as expected?

The collapsed grid looks fine, but once expanded, the item moves to the next row - is that expected?
collapsed

expanded

The video tiles don't work - those are just placeholders?

issackjohn · 2024-12-10T17:47:15Z

Some visual questions - these might not be relevant to the tests:

Is this spacing as expected?

I'll add a fix for that. The webpage was designed to be viewed in 800x600, as that's how it will appear within the benchmark.

The collapsed grid looks fine, but once expanded, the item moves to the next row - is that expected? collapsed

Yes, that is expected. Do we prefer a different behavior?

The video tiles don't work - those are just placeholders?

Yes, they are just placeholders. We intended the video elements to be there but did not plan for them to play any actual videos.

flashdesignory · 2024-12-10T18:50:53Z

Some visual questions - these might not be relevant to the tests:
Is this spacing as expected?

I'll add a fix for that. The webpage was designed to be viewed in 800x600, as that's how it will appear within the benchmark.

Thanks - I'd vote for supporting larger viewport sizes. This will ensure that the workload looks usable at larger viewports, in case we want to change what's supported in Speedometer later on.

The collapsed grid looks fine, but once expanded, the item moves to the next row - is that expected? collapsed

Yes, that is expected. Do we prefer a different behavior?

I guess it makes sense for the tests, since it shifts elements on the page. That said, it's not something you'd see on the web (hopefully). I'd expected a modal or something like that instead.

The video tiles don't work - those are just placeholders?

Yes, they are just placeholders. We intended the video elements to be there but did not plan for them to play any actual videos.

Thanks for clarifying.

flashdesignory · 2024-12-10T18:51:40Z

@issackjohn - maybe rebase / merge main, to get the updated test runner in there. That should ensure the checks don't fail.

camillobruni · 2024-12-10T19:16:45Z

Let's aim for > 5ms for each measured subtest (I'd still aim for roughly 10ms if possible). Does this sound reasonable?

issackjohn · 2024-12-11T17:08:31Z

@issackjohn - maybe rebase / merge main, to get the updated test runner in there. That should ensure the checks don't fail.

Sounds good.

issackjohn · 2024-12-11T17:09:55Z

Let's aim for > 5ms for each measured subtest (I'd still aim for roughly 10ms if possible). Does this sound reasonable?

Sounds reasonable to me, I'll add some more actions.

camillobruni · 2024-12-17T16:38:49Z

Just double checked on Chrome, Firefox, Safari and the noise levels are in-sync with the rest of the workloads.
The minimum step times look good now I would say.

issackjohn · 2025-01-22T19:27:25Z

Thanks! Looking forward to addressing anymore feedback from folks.

netlify · 2025-02-11T17:50:04Z

✅ Deploy Preview for webkit-speedometer ready!

Name	Link
🔨 Latest commit	`698c725`
🔍 Latest deploy log	https://app.netlify.com/sites/webkit-speedometer/deploys/67d4a0f54b21080008d5e50e
😎 Deploy Preview	https://deploy-preview-453--webkit-speedometer.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

julienw

Ths looks pretty good to me!

I see that when running npm run dev the dist files get overridden. Do you know if we can make it so the dev files are written in a different location, to minimize risks of errors in the future?

Otherwise my main issue is that the width setting in ReduceWidthIn5Steps and IncreaseWidthIn5Steps don't seem to be captured properly, at least in Firefox on my current machine (which is a bit slow). IMO this needs to be fixed, but this can be done in a follow-up.

Then I have a few more minor comments here and there. I haven't looked very closely at the tailwind code because I'm not familiar with it.
Also I wonder if we could include matchMedia somewhere.

Thanks!

Please add new changes in a separate commit.

julienw · 2025-01-14T14:12:55Z

.gitignore

@@ -35,3 +35,4 @@

 /resources/charts/node_modules/
 /resources/todomvc/big-dom-generator/node_modules/
+/experimental/responsive-design/node_modules/


at this point, we should just add node_modules which would ignore all directories name that way :-) (but that can be done in a follow-up PR)

julienw · 2025-01-14T14:13:26Z

experimental/responsive-design/.prettierignore

+src/tailwind.generated.css
+src/tailwind.chat-window.generated.css


maybe use *.generated.css?

julienw · 2025-01-14T14:14:26Z

.prettierignore

@@ -46,3 +46,5 @@ package-lock.json
 /resources/perf.webkit.org

 /resources/react-stockcharts
+
+/experimental/responsive-design/


I'd remove that, we want the workload to follow the same code style.

The 2 existing ignores are there mostly because they're dumps of existing projects. (I'm not sure this is a good reason, but well that's the history)

julienw · 2025-01-14T14:14:56Z

experimental/responsive-design/.prettierrc

+{
+    "arrowParens": "always",
+    "bracketSameLine": false,
+    "bracketSpacing": true,
+    "htmlWhitespaceSensitivity": "css",
+    "printWidth": 250,
+    "semi": true,
+    "singleQuote": false,
+    "tabWidth": 4,
+    "trailingComma": "es5",
+    "useTabs": false,
+    "plugins": ["prettier-plugin-tailwindcss"]
+}


let's use the project-wide prettier configuration
(should be changed soon finally)

but if you want to do it after #371 that's fine by me too.

julienw · 2025-01-14T14:15:37Z

experimental/responsive-design/eslint.config.js

@@ -0,0 +1,74 @@
+export default [


let's use the project-wide eslint configuration

julienw · 2025-01-14T14:19:27Z

experimental/responsive-design/package.json

+        "dev:rollup": "rollup --config rollup.config.js --watch",
+        "dev:serve": "web-dev-server --watch",
+        "lint:check": "eslint **/*.{js,mjs,jsx,ts,tsx}",
+        "lint:fix": "eslint \"**/*.{js,mjs,jsx,ts,tsx}\" --fix",


nit:

Suggested change

"lint:fix": "eslint \"**/*.{js,mjs,jsx,ts,tsx}\" --fix",

"lint:fix": "npm lint:check -- --fix",

julienw · 2025-04-17T16:51:25Z

experimental/responsive-design/src/lib/components/app-ribbon.js

+        const breakpoint = breakpoints.find((bp) => width >= bp.minWidth);
+        this.visibleButtons = breakpoint ? this.buttons.slice(0, breakpoint.buttons) : this.buttons.slice(0, 2);


I'm curious, why it's not done with CSS or window.matchMedia?
Is it just to exercize ResizeObserver?
Can we please have a comment to explain that? (also in the other file where it's used)

julienw · 2025-04-17T17:00:44Z

resources/benchmark-runner.mjs

-    querySelector(selector, path = []) {
-        const lookupStartNode = this._frame.contentDocument;
+    querySelector(selector, path = [], inIframe = false) {
+        const lookupStartNode = inIframe ? this._frame.contentDocument : this._frame.contentDocument;


The 2 parts of the ternary are identical, is that expected?

julienw · 2025-04-17T17:15:27Z

resources/tests.mjs

+
+            widths.forEach((width) => {
+                iframeElement.setWidth(`${width}px`);
+                page.layout();


I think the layout doesn't work, possibly because the resize observer is async?
I think you should try doing several steps instead of just one, so that the async part is properly captured.

julienw · 2025-04-17T17:17:07Z

resources/tests.mjs

+            const widths = [560, 640, 704, 768, 800];
+
+            widths.forEach((width) => {
+                iframeElement.setWidth(`${width}px`);


issackjohn force-pushed the responsive-design branch 2 times, most recently from 1da5897 to 34a545e Compare November 15, 2024 19:19

issackjohn requested review from julienw and removed request for julienw November 15, 2024 19:22

issackjohn marked this pull request as ready for review November 15, 2024 19:22

issackjohn requested review from camillobruni, julienw, flashdesignory, rniwa and bgrins November 15, 2024 19:27

camillobruni reviewed Dec 3, 2024

View reviewed changes

issackjohn force-pushed the responsive-design branch from cd96d0b to 3413c3d Compare December 11, 2024 17:36

issackjohn force-pushed the responsive-design branch from 057e604 to e7887e9 Compare February 11, 2025 17:49

workload

698c725

issackjohn force-pushed the responsive-design branch from e7887e9 to 698c725 Compare March 14, 2025 21:34

julienw requested changes Apr 17, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Experimental Workload: Responsive Design #453

Experimental Workload: Responsive Design #453

issackjohn commented Nov 15, 2024 •

edited

Loading

issackjohn commented Nov 15, 2024

rniwa commented Nov 15, 2024

issackjohn commented Nov 15, 2024

rniwa commented Nov 15, 2024

issackjohn commented Dec 2, 2024

camillobruni commented Dec 3, 2024

camillobruni Dec 3, 2024

issackjohn Dec 4, 2024

issackjohn commented Dec 5, 2024

flashdesignory commented Dec 10, 2024

issackjohn commented Dec 10, 2024

flashdesignory commented Dec 10, 2024

flashdesignory commented Dec 10, 2024

camillobruni commented Dec 10, 2024

issackjohn commented Dec 11, 2024

issackjohn commented Dec 11, 2024

camillobruni commented Dec 17, 2024

issackjohn commented Jan 22, 2025

netlify bot commented Feb 11, 2025 •

edited

Loading

julienw left a comment

julienw Jan 14, 2025

julienw Jan 14, 2025

julienw Jan 14, 2025

julienw Jan 14, 2025

julienw Apr 17, 2025

julienw Jan 14, 2025

julienw Jan 14, 2025

julienw Apr 17, 2025

julienw Apr 17, 2025

julienw Apr 17, 2025

julienw Apr 17, 2025

		src/tailwind.generated.css
		src/tailwind.chat-window.generated.css

	"lint:fix": "eslint \"*/.{js,mjs,jsx,ts,tsx}\" --fix",
	"lint:fix": "npm lint:check -- --fix",

		const breakpoint = breakpoints.find((bp) => width >= bp.minWidth);
		this.visibleButtons = breakpoint ? this.buttons.slice(0, breakpoint.buttons) : this.buttons.slice(0, 2);

Experimental Workload: Responsive Design #453

Are you sure you want to change the base?

Experimental Workload: Responsive Design #453

Conversation

issackjohn commented Nov 15, 2024 • edited Loading

issackjohn commented Nov 15, 2024

rniwa commented Nov 15, 2024

issackjohn commented Nov 15, 2024

rniwa commented Nov 15, 2024

issackjohn commented Dec 2, 2024

camillobruni commented Dec 3, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

issackjohn commented Dec 5, 2024

flashdesignory commented Dec 10, 2024

issackjohn commented Dec 10, 2024

flashdesignory commented Dec 10, 2024

flashdesignory commented Dec 10, 2024

camillobruni commented Dec 10, 2024

issackjohn commented Dec 11, 2024

issackjohn commented Dec 11, 2024

camillobruni commented Dec 17, 2024

issackjohn commented Jan 22, 2025

netlify bot commented Feb 11, 2025 • edited Loading

✅ Deploy Preview for webkit-speedometer ready!

julienw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

issackjohn commented Nov 15, 2024 •

edited

Loading

netlify bot commented Feb 11, 2025 •

edited

Loading