Closed Bug 1350053 Opened 5 years ago Closed 5 years ago

Install flow of the new static themes does not match that of LWTs

Categories

(WebExtensions :: Frontend, defect)

defect
Not set
major

Tracking

(firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: krupa--use, Assigned: mikedeboer)

References

Details

Attachments

(4 files)

55.0a1 (2017-03-23) (64-bit)

steps to reproduce:
1. On Nightly, set pref extensions.webextensions.themes.enabled to true
2. Go to https://addons.mozilla.org/en-US/firefox/themes/?sort=popular and install any of the old-style LWTs
3. Notice that after the install is successful there is a notification bar at the top with options to Undo/Manage Themes.
4. Install a new static theme and notice the install flow

expected behavior:
install flow for the new static themes is similar to old-style LWTs

actual behavior:
install flow for the new static themes is exactly like a webextension which may not be apt for themes.

screencast:
https://www.dropbox.com/s/242mgg5nlulz8y1/installflow.mov?dl=0
Krupa, how did you create that .xpi? Can you tell me how you created it or point me to a wiki page? Thanks!
Status: NEW → ASSIGNED
Flags: needinfo?(kraj)
Steps to create a new-style static theme:
1. Create a manifest.json using the sample listed in https://docs.google.com/document/d/1ueD6V7aLLTuc1GAOxxQYcwl2HR-k62HHu3q8knTJ4FU/edit?pli=1#heading=h.bij4y75jj419
2. Link to the relevant images in the folder which is zipped and then converted to an xpi (header image was copied from an existing LWT)
3. Have the .xpi file signed by uploading it as an unlisted add-on (you can skip this step on Nightly by turning the signing off)

Let me know if these steps are clear enough.
Flags: needinfo?(kraj)
Dave, before I update all the tests, what do you think of this approach?

I wanted LWTs to adopt the WebExtensions Themes flow, because retro-fitting felt like a bad idea and unnecessary maintenance burden.
Attachment #8855812 - Flags: feedback?(dtownsend)
Comment on attachment 8855812 [details] [diff] [review]
WIP Patch: Change install flow of LWTs to use PopupNotifications

Review of attachment 8855812 [details] [diff] [review]:
-----------------------------------------------------------------

The approach seems fine however I think you'll need to rev the l10n ids since you're displaying them in a different context.

Has UX signed off on this change though?
Attachment #8855812 - Flags: feedback?(dtownsend) → feedback+
Comment on attachment 8855812 [details] [diff] [review]
WIP Patch: Change install flow of LWTs to use PopupNotifications

Hi Markus, can you review this change by watching the following two videos:

 - Status Quo - https://vimeo.com/212581150
 - Now using doorhangers - https://vimeo.com/212581162

?

Please let me know if you have any further questions! (I requested a ScreenFlow license again, so next time they will be more pro-quality ;-) )
Attachment #8855812 - Flags: ui-review?(mjaritz)
Comment on attachment 8855812 [details] [diff] [review]
WIP Patch: Change install flow of LWTs to use PopupNotifications

Looks great! I love that you got rid of the notification bar! Thank you.
Attachment #8855812 - Flags: ui-review?(mjaritz) → ui-review+
Comment on attachment 8856591 [details]
Bug 1350053 - Part 1 - Change install flow of Light-weight Themes (LWT) to use PopupNotifications, instead of notification boxes.

https://reviewboard.mozilla.org/r/128546/#review130994
Attachment #8856591 - Flags: review?(dtownsend) → review+
Comment on attachment 8856592 [details]
Bug 1350053 - Part 2 - Update tests to check the arrow panel to appear, instead of the notification bar.

https://reviewboard.mozilla.org/r/128548/#review130996
Attachment #8856592 - Flags: review?(dtownsend) → review+
Pushed by mdeboer@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5c0565ed332f
Part 1 - Change install flow of Light-weight Themes (LWT) to use PopupNotifications, instead of notification boxes. r=mossop
https://hg.mozilla.org/integration/autoland/rev/74d44ad1533e
Part 2 - Update tests to check the arrow panel to appear, instead of the notification bar. r=mossop
https://hg.mozilla.org/mozilla-central/rev/5c0565ed332f
https://hg.mozilla.org/mozilla-central/rev/74d44ad1533e
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Toolkit → WebExtensions
You need to log in before you can comment on or make changes to this bug.