Page MenuHomePhabricator

Migrate QuickSurveys from WVUI to Codex
Closed, ResolvedPublic

Description

WVUI is going to be deprecated and removed. QuickSurveys uses WVUI and is deployed in production, so it needs to be migrated to Codex before the WVUI deprecation can happen.

All WVUI components used in QuickSurveys already have equivalents in Codex:

  • Button
  • Checkbox
  • Icon
  • TextInput

Event Timeline

Change 769524 had a related patch set uploaded (by Catrope; author: Jdlrobson):

[mediawiki/extensions/QuickSurveys@master] QuickSurveys uses Codex instead of WVUI

https://gerrit.wikimedia.org/r/769524

The folllowing is from a routine perf analysis of a running survey. Perhaps some of it is of use here:

Notes from comparing these two (logged-out, after 1 warmup and dismissing CentralNotice, Chrome, 3G Fast, CPU unthrottled):

First, from a functional perspective I notice: […]

A quick analysis from a perf timeline capture:

  • The vue+wvui payload is okay-ish, and what we know and expect/tolerate for sampled/temporary campaigns:
    • .. transfer 95kB on page load,
    • .. compile 298 kB of code after decompession.
    • .. 4ms (unthrottled) to define and export Vue.
    • .. 6ms (unthrottled) to define and export WVUI.
  • The ext.quicksurveys.lib/vue/ render() function
    • .. takes ~40ms to sychronously execute when unthrottled on a MacBook Pro in Chrome. That's a lot of raw compute to concatenate and parse a 0.9kB string of HTML. I expect that on more representative hardware, it easily goes to 100ms or more beyond the 50ms long task threshold.
    • .. a notable portion of this time is spent creating and manipulating a DOM hierachy for survey.link message, which is then discarded and casted to a string fed to the URL constructor. This looks to me like a bug as presumably we do not intend to support HTML or wikitext syntax inside a URL.
var externalLink = survey.link ? new mw.Uri( mw.message( survey.link ).parse() ) : '';

I thought perhaps it uses parse() as hack to support $N-message parameters, but no parameters are passed (And that would normally be done via mw.message().text, mw.msg, or mw.format). Removing that would shave off 10% of the Vue rendering time.

capture.png (1×2 px, 288 KB)

Change 769524 merged by jenkins-bot:

[mediawiki/extensions/QuickSurveys@master] QuickSurveys uses Codex instead of WVUI

https://gerrit.wikimedia.org/r/769524

Change 815801 had a related patch set uploaded (by Jdlrobson; author: Jdlrobson):

[mediawiki/extensions/QuickSurveys@master] Pass event correctly

https://gerrit.wikimedia.org/r/815801

Change 815801 merged by jenkins-bot:

[mediawiki/extensions/QuickSurveys@master] Logging: Restore logging of responses

https://gerrit.wikimedia.org/r/815801

Jdlrobson triaged this task as Medium priority.Jul 21 2022, 4:57 PM

I checked the EventLogging portion on https://en.wikipedia.beta.wmflabs.org/wiki/Dog?quicksurvey=true

I can see an initiation event:
{F35328696}

and a response event:
{F35328698}

I'm not seeing any visual regressions. The only ones flagged by https://pixel.wmcloud.org/reports/web-maintained/index.html relate to T261391

The only surveys I see are button-types. Would be good to know how to trigger surveys that have other component types as specified in the task description.

image.png (918×1 px, 423 KB)

Thanks @Jdlrobson!

Looks great. Any luck you know where I can get on with an icon?

No problem! The icon is the X in the top right corner!

It was right under my nose! I was thinking along the lines of an icon on a button. Thanks!

Let us know if you need any help from web in signing this off.

DAbad changed the task status from Open to In Progress.Sep 1 2022, 3:23 PM

Product sign-off complete.

Am I understanding correctly that QuickSurveys can only be used on ES6 browsers now? If so, we should update the extension documentation and metadata—I came across this issue because we're seeing CI errors on all patches in QuickSurveys:

09:43:33 1) ResourcesTest::testUnsatisfiableDependencies
09:43:33 The module 'ext.quicksurveys.lib.vue' must not depend on modules with requiresES6=true
09:43:33 Failed asserting that two arrays are equal.
09:43:33 --- Expected
09:43:33 +++ Actual
09:43:33 @@ @@
09:43:33  Array (
09:43:33 +    0 => '@wikimedia/codex'
09:43:33  )

Also, are we okay with the ongoing perceived performance survey only appearing on ES6 browsers?

Change 832459 had a related patch set uploaded (by Awight; author: Awight):

[mediawiki/extensions/QuickSurveys@master] Require ES6 for client code in this repo

https://gerrit.wikimedia.org/r/832459

Change 832459 merged by jenkins-bot:

[mediawiki/extensions/QuickSurveys@master] Require ES6 for client code in this repo

https://gerrit.wikimedia.org/r/832459

Am I understanding correctly that QuickSurveys can only be used on ES6 browsers now?

Yes that's correct for any code using Codex/Vue now.