Add experimental interop mobile results view #3959

DanielRyanSmith · 2024-08-26T00:53:47Z

This change pulls mobile results data from the interop-results repo and uses it to display a new mobile results view on the Interop dashboard. The mobile results view can only be seen by toggling a flag in /flags.

Screen.Recording.2024-10-15.at.4.38.33.PM.mov

DanielRyanSmith · 2024-08-26T18:05:29Z

It looks like the linter is failing due to an issue unrelated to this change.

api/query/cache/index/index.go:210:18: G115: integer overflow conversion uint64 -> int (gosec)
		shardIdx := int(t.testID % numShardsU64)
		               ^
api/query/cache/backfill/backfill.go:120:14: G115: integer overflow conversion uint64 -> int (gosec)
	limit := int(maxBytes/bytesPerRun) / len(shared.GetDefaultProducts())

securego/gosec#1149

KyleJu · 2024-08-26T18:09:36Z

It looks like the linter is failing due to an issue unrelated to this change.

api/query/cache/index/index.go:210:18: G115: integer overflow conversion uint64 -> int (gosec)
		shardIdx := int(t.testID % numShardsU64)
		               ^
api/query/cache/backfill/backfill.go:120:14: G115: integer overflow conversion uint64 -> int (gosec)
	limit := int(maxBytes/bytesPerRun) / len(shared.GetDefaultProducts())

securego/gosec#1149

I will get to it shortly

KyleJu · 2024-08-26T18:59:08Z

It looks like the linter is failing due to an issue unrelated to this change.

api/query/cache/index/index.go:210:18: G115: integer overflow conversion uint64 -> int (gosec)
		shardIdx := int(t.testID % numShardsU64)
		               ^
api/query/cache/backfill/backfill.go:120:14: G115: integer overflow conversion uint64 -> int (gosec)
	limit := int(maxBytes/bytesPerRun) / len(shared.GetDefaultProducts())

securego/gosec#1149

I will get to it shortly

Sent out #3968

DanielRyanSmith · 2024-08-27T21:42:51Z

View the staging deployment here: https://2024-08-25-mobile-resu-dot-wptdashboard-staging.uk.r.appspot.com/interop-2024?mobileView

It might be worth changing this view in some way to be even more explicit that these are mobile results and experimental results.

jgraham · 2024-09-10T15:29:42Z

I don't have detailed code-level feedback, but:

In general I approve the idea of landing something like this in some hidden way (query parameter, flag, etc.)
Even so, I think the page needs explicit text explaining that the mobile work remains a WIP and the scores may not reflect the real level of support for a given feature (don't want people to say "look how the mobile web sucks; browsers are X% worse on interop on mobile" if it's just test infra problems)

DanielRyanSmith · 2024-10-16T22:39:26Z

Some odd logic has been added in a commit that can be reverted later:

9068283

Once we have Safari results to populate the page

KyleJu

LGTM with some minor comments

KyleJu · 2024-10-17T17:19:20Z

webapp/components/wpt-flags.js

    </paper-item>
+    <paper-item>
+      <paper-checkbox checked="{{showMobileScoresView}}">
+        Allow mobile results view on Interop dashboard


Enable or Show mobile results view on Interop dashboard to make the wording consistent?

I like it better. Changed!

KyleJu · 2024-10-17T17:25:18Z

webapp/interop_handler.go

 	}

+	q := r.URL.Query()
+	isMobileView, err := shared.ParseBooleanParam(q, "mobileView")


It is recommended to use hyphens to separate words [1]. It is also consistent with API params here [2]

[1] https://developers.google.com/search/docs/crawling-indexing/url-structure
[2] https://developers.google.com/search/docs/crawling-indexing/url-structure

Good call - I'll change to use that syntax

webapp/interop_handler_test.go

webapp/components/interop-dashboard.js

KyleJu · 2024-10-17T17:28:46Z

webapp/components/interop-dashboard.js

+          </interop-summary>
        </div>
        <div class="grid-item grid-item-description">
          <p>Interop [[year]] is a cross-browser effort to improve the interoperability of the web —


Suggested change

<p>Interop [[year]] is a cross-browser effort to improve the interoperability of the web —

<p>Interop [[year]] is a cross-browser effort to improve the interoperability of the web -

Is this a hyphen? It looks really long

This is an em dash and was chosen to use by the team that wrote up this dashboard description

Aha thanks!

webapp/components/interop-dashboard.js

KyleJu · 2024-10-17T19:22:18Z

webapp/components/interop-data-manager.js


        let testScore = 0.0;
+        // Mobile csv does not have an Interop version column to account for.
+        let versionOffset = (this.isMobileScoresView && browserName === 'Interop') ? 0 : 1;


Suggested change

let versionOffset = (this.isMobileScoresView && browserName === 'Interop') ? 0 : 1;

const versionOffset = (this.isMobileScoresView && browserName === 'Interop') ? 0 : 1;

KyleJu · 2024-10-17T19:28:17Z

webapp/interop_handler.go

+	_, isValidMobileYear := validMobileYears[year];
+	if isMobileView != nil && !isValidMobileYear {
+		year = defaultRedirectYear
+		needsRedirect = true


minor detail: should it redirect to interop-2024? or interop-2024?mobile-view?

Currently, it maintains all query parameters, so e.g. /interop-2023?mobile-view will redirect to /interop-2024?mobile-view

I don't think that is the case testing on the staging deployment - https://2024-08-25-mobile-resu-dot-wptdashboard-staging.uk.r.appspot.com/interop-2023?mobile-view

nm it worked! The staging deployment was not up-to-date, https://2024-08-25-mobile-resu-dot-wptdashboard-staging.uk.r.appspot.com/interop-2023?mobile-view. Thanks!

DanielRyanSmith and others added 4 commits August 27, 2024 14:04

add mobile results view for 2024

27728a2

lint

49303ad

handle mobile view redirects

75a1a3b

interop JSON data matches

7523fdf

DanielRyanSmith force-pushed the 2024-08-25_mobile-results-view branch from c07b579 to 7523fdf Compare August 27, 2024 21:04

jcscottiii mentioned this pull request Sep 10, 2024

Agenda 2024-09-10 web-platform-tests/interop-mobile-testing#30

Open

past closed this Sep 17, 2024

past reopened this Sep 17, 2024

DanielRyanSmith added 5 commits October 7, 2024 17:33

Add text about mobile scores being WIP

0cda0d2

Put mobile results view behind a flag

f514add

handle stable click properly

27806aa

Add Safari iOS as a placeholder

9068283

lint

b8042d1

DanielRyanSmith requested a review from KyleJu October 16, 2024 22:37

KyleJu approved these changes Oct 17, 2024

View reviewed changes

changes suggested by @KyleJu

cd5eb5c

DanielRyanSmith merged commit aa11f43 into main Oct 21, 2024
12 checks passed

DanielRyanSmith deleted the 2024-08-25_mobile-results-view branch October 21, 2024 18:25

DanielRyanSmith mentioned this pull request Oct 21, 2024

Deploy fa3d19e6 to production #4082

Closed

	<p>Interop [[year]] is a cross-browser effort to improve the interoperability of the web —
	<p>Interop [[year]] is a cross-browser effort to improve the interoperability of the web -

	let versionOffset = (this.isMobileScoresView && browserName === 'Interop') ? 0 : 1;
	const versionOffset = (this.isMobileScoresView && browserName === 'Interop') ? 0 : 1;

Add experimental interop mobile results view #3959

Add experimental interop mobile results view #3959

Uh oh!

Conversation

DanielRyanSmith commented Aug 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DanielRyanSmith commented Aug 26, 2024

Uh oh!

KyleJu commented Aug 26, 2024

Uh oh!

KyleJu commented Aug 26, 2024

Uh oh!

DanielRyanSmith commented Aug 27, 2024

Uh oh!

jgraham commented Sep 10, 2024

Uh oh!

DanielRyanSmith commented Oct 16, 2024

Uh oh!

KyleJu left a comment

Choose a reason for hiding this comment

Uh oh!

KyleJu Oct 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

DanielRyanSmith commented Aug 26, 2024 •

edited

Loading

KyleJu Oct 17, 2024 •

edited

Loading