2034623 - Create asrouter-newtab-multistage component that allows for embedding multistage messages inline in newtab

Status: RESOLVED FIXED

(Firefox :: Messaging System, enhancement, P1)

Description

[Mike Conley (:mconley) (:⚙️)]

about:welcome has these really nice multistage messages via the MultiStageAboutWelcome component. I'd like to try adapting this component so that it's less tied to about:welcome, and can be embedded in about:newtab.

I spoke to emcminn and aminomancer and got a high-level okay to dig into this, since part of this is moving a bunch of React component definitions out from browser/components/aboutwelcome to browser/components/asrouter instead.

Comment 1

[Mike Conley (:mconley) (:⚙️)]

pdahiya also expressed interest in what I'm doing here.

Comment 2

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 2034623 - Fix ExternalComponentWrapper remounting on every re-render r?#home-newtab-reviewers!

The default value for importModule was an inline arrow function defined
inside the component's parameter list. JavaScript creates a new function
object each time the component renders, so React's useEffect dependency
comparison always saw importModule as changed, running the effect's
cleanup (which tears down the custom element or unmounts the React
bundle) followed by a full reload on every parent re-render.

Hoisting the default to a module-level constant gives it a stable
identity across renders, so the effect only re-runs when type or
components actually change.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 2034623 - Move about:welcome components out into browser/components/asrouter to be more obviously reusable. r?#omc-reviewers!

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 2034623 - Rename AWScreenUtils to ASRouterScreenUtils and move to browser/components/asrouter. r?#omc-reviewers!

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 2034623 - Stub out a new asrouter-newtab-multistage component and bundle, and package it to train-hop. r?#omc-reviewers!,#home-newtab-reviewers!

Comment 6

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 2034623 - Embed asrouter-newtab-multistage message in newtab. r?#omc-reviewers!,#home-newtab-reviewers!

Comment 7

[Mike Conley (:mconley) (:⚙️)]

Attachment: Bug 2034623 - Add mochitests for asrouter-newtab-multistage embedding in newtab. r?#omc-reviewers!

Comment 8

[Mike Conley (:mconley) (:⚙️)]

Attachment: WIP: Bug 2034623 - Add test for browser_asrouter_newtab_multistage messages.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Attachment: image.png

Notably, New Tab stories must be disabled in order for this message to appear, but otherwise, kinda incredibly, this seems to work.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

To help unblock / avoid bitrotting other patches under browser/components/aboutwelcome and browser/components/asrouter, I'm going to attempt to land the first 3 reviewed patches, which move a bunch of things around in preparation for the multistage message component.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7ad096607219
https://hg.mozilla.org/integration/autoland/rev/233dfb424f2b
Fix ExternalComponentWrapper remounting on every re-render r=home-newtab-reviewers,thecount
https://github.com/mozilla-firefox/firefox/commit/043e5e016b26
https://hg.mozilla.org/integration/autoland/rev/a1c73fb102ca
Move about:welcome components out into browser/components/asrouter to be more obviously reusable. r=omc-reviewers,frontend-codestyle-reviewers,mimi
https://github.com/mozilla-firefox/firefox/commit/f5acb4ac9f74
https://hg.mozilla.org/integration/autoland/rev/165cf6e0cb88
Rename AWScreenUtils to ASRouterScreenUtils and move to browser/components/asrouter. r=omc-reviewers,mimi

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ae2c8ef62407
https://hg.mozilla.org/integration/autoland/rev/72a4c8a3af45
Revert "Bug 2034623 - Rename AWScreenUtils to ASRouterScreenUtils and move to browser/components/asrouter. r=omc-reviewers,mimi" for causing node newtab failures

Comment 13

[Sandor Molnar[:smolnar]]

Backed out for causing node newtab failures

Backout link: https://hg.mozilla.org/integration/autoland/rev/72a4c8a3af4531669d09bd16195f8ed14c1aa13e

Push with failures

Failures log ->

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e4b689a8c116
https://hg.mozilla.org/integration/autoland/rev/f41156ccb6a1
Fix ExternalComponentWrapper remounting on every re-render r=home-newtab-reviewers,thecount
https://github.com/mozilla-firefox/firefox/commit/1a2ff4f8a0e5
https://hg.mozilla.org/integration/autoland/rev/2065a7d8c06a
Move about:welcome components out into browser/components/asrouter to be more obviously reusable. r=omc-reviewers,frontend-codestyle-reviewers,mimi
https://github.com/mozilla-firefox/firefox/commit/49ed13234631
https://hg.mozilla.org/integration/autoland/rev/f438135bcfc1
Rename AWScreenUtils to ASRouterScreenUtils and move to browser/components/asrouter. r=omc-reviewers,mimi

Comment 15

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/f41156ccb6a1
https://hg.mozilla.org/mozilla-central/rev/2065a7d8c06a
https://hg.mozilla.org/mozilla-central/rev/f438135bcfc1

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2b99c1a0ff99
https://hg.mozilla.org/integration/autoland/rev/1e67a84ba6f3
Stub out a new asrouter-newtab-multistage component and bundle, and package it to train-hop. r=omc-reviewers,home-newtab-reviewers,frontend-codestyle-reviewers,mimi,ini
https://github.com/mozilla-firefox/firefox/commit/9a2d5e42c285
https://hg.mozilla.org/integration/autoland/rev/1d864ccf1a23
Embed asrouter-newtab-multistage message in newtab. r=omc-reviewers,home-newtab-reviewers,mimi,ini
https://github.com/mozilla-firefox/firefox/commit/1cb38170d24c
https://hg.mozilla.org/integration/autoland/rev/cb027f12df23
Add mochitests for asrouter-newtab-multistage embedding in newtab. r=omc-reviewers,emcminn

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/847eb905466a
https://hg.mozilla.org/integration/autoland/rev/a10d01c287a8
Revert "Bug 2034623 - Add mochitests for asrouter-newtab-multistage embedding in newtab. r=omc-reviewers,emcminn" for causing bc failures @browser_asrouter_newtab_multistage_messages.js.

Comment 18

[agoloman]

Backed out for causing bc failures @browser_asrouter_newtab_multistage_messages.js.

• Backout link
• Push with failures
• Failure Log

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5190848174c3
https://hg.mozilla.org/integration/autoland/rev/40907503d2d9
Stub out a new asrouter-newtab-multistage component and bundle, and package it to train-hop. r=omc-reviewers,home-newtab-reviewers,frontend-codestyle-reviewers,mimi,ini
https://github.com/mozilla-firefox/firefox/commit/ffadc5d27757
https://hg.mozilla.org/integration/autoland/rev/3428ba052090
Embed asrouter-newtab-multistage message in newtab. r=omc-reviewers,home-newtab-reviewers,mimi,ini
https://github.com/mozilla-firefox/firefox/commit/9c645ae22578
https://hg.mozilla.org/integration/autoland/rev/06a287ff2e0d
Add mochitests for asrouter-newtab-multistage embedding in newtab. r=omc-reviewers,emcminn

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/7d103428974b
https://hg.mozilla.org/integration/autoland/rev/dc98c5984d59
Revert "Bug 2034623 - Add mochitests for asrouter-newtab-multistage embedding in newtab. r=omc-reviewers,emcminn" for causing bc failures @browser_parsable_css.js.

Comment 21

[agoloman]

Backed out for causing bc failures @browser_parsable_css.js.

• Backout link
• Push with failures
• Failure Log

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f2a53b040236
https://hg.mozilla.org/integration/autoland/rev/c6523682d936
Stub out a new asrouter-newtab-multistage component and bundle, and package it to train-hop. r=omc-reviewers,home-newtab-reviewers,frontend-codestyle-reviewers,mimi,ini
https://github.com/mozilla-firefox/firefox/commit/7afb230a292c
https://hg.mozilla.org/integration/autoland/rev/11815a141575
Embed asrouter-newtab-multistage message in newtab. r=omc-reviewers,home-newtab-reviewers,mimi,ini
https://github.com/mozilla-firefox/firefox/commit/c0a337153a23
https://hg.mozilla.org/integration/autoland/rev/98c68a74e7d1
Add mochitests for asrouter-newtab-multistage embedding in newtab. r=omc-reviewers,emcminn

Comment 23

[Silaghi Andreea]

https://hg.mozilla.org/mozilla-central/rev/c6523682d936
https://hg.mozilla.org/mozilla-central/rev/11815a141575
https://hg.mozilla.org/mozilla-central/rev/98c68a74e7d1