Closed Bug 1392469 Opened 5 years ago Closed 5 years ago

[Onboarding] remove fox icon and speech bubble from notification

Categories

(Firefox :: New Tab Page, enhancement, P1)

57 Branch
enhancement

Tracking

()

VERIFIED FIXED
Firefox 57
Tracking Status
firefox57 --- verified

People

(Reporter: gasolin, Assigned: gasolin)

References

Details

(Whiteboard: [photon-onboarding][photon-onboarding-newui])

Attachments

(2 files, 1 obsolete file)

* remove fox icon and speech bubble from notification
* remove all notification illustrations
Whiteboard: [photon-onboarding][triage]
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
bug 1371538 doesn't need to include the illustration for notification after this UI update is landed.
Blocks: 1371538
Flags: qe-verify+
Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

https://reviewboard.mozilla.org/r/171968/#review177194

::: browser/extensions/onboarding/content/onboarding.css:536
(Diff revision 1)
> -@media all and (max-width: 960px) {
> -  #onboarding-notification-icon {
> -    display: none;
> -  }
> -}
>  @media all and (max-width: 720px) {

I think we don't need this either but can be done in bug 1392552
Priority: P2 → P1
QA Contact: jwilliams
Attachment #8900566 - Attachment is obsolete: true
Attachment #8900566 - Flags: review?(fliu)
Blocks: 1392468
Comment on attachment 8900584 [details]
Bug 1392469 - [Onboarding] remove all notification illustrations;

https://reviewboard.mozilla.org/r/171982/#review177304
Attachment #8900584 - Flags: review?(fliu) → review+
found icon64.png fit 64x64 size well, so replace it instead of about-logo@2x.png
Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

https://reviewboard.mozilla.org/r/171968/#review177310

r+ with issues addressed, thanks

::: browser/extensions/onboarding/content/onboarding.js
(Diff revision 5)
>          <button id="onboarding-notification-action-btn"></button>
>        </section>
>        <button id="onboarding-notification-close-btn" class="onboarding-close-btn"></button>
>      `;
> -    let toolTip = this._bundle.formatStringFromName(
> -      this._tourType === "new" ? "onboarding.notification-icon-tool-tip" :

If these strings were no longer needed, please remove them from the properties file.

::: browser/extensions/onboarding/test/browser/browser_onboarding_accessibility.js:47
(Diff revision 5)
>    await ContentTask.spawn(tab.linkedBrowser, {}, function() {
>      let doc = content.document;
>      let footer = doc.getElementById("onboarding-notification-bar");
> -    let icon = doc.getElementById("onboarding-notification-icon")
> +    let title = doc.getElementById("onboarding-notification-tour-title");
> +
> +    is(footer.getAttribute("aria-labelledby"), title.id,

doc.getElementById("onboarding-notification-tour-title").id is always "onboarding-notification-tour-title" so maybe title.id could be removed.
Attachment #8900565 - Flags: review?(fliu) → review+
Comment on attachment 8900565 [details]
Bug 1392469 - [Onboarding] remove fox icon and speech bubble from notification;

https://reviewboard.mozilla.org/r/171968/#review177310

> If these strings were no longer needed, please remove them from the properties file.

nice catch! updated

> doc.getElementById("onboarding-notification-tour-title").id is always "onboarding-notification-tour-title" so maybe title.id could be removed.

updated
Pushed by flin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/baeaa44166fb
[Onboarding] remove fox icon and speech bubble from notification;r=Fischer
https://hg.mozilla.org/integration/autoland/rev/5da4c3f0fd7c
[Onboarding] remove all notification illustrations;r=Fischer
I have verified that this issue is fixed on today's nightly.
Status: RESOLVED → VERIFIED
Whiteboard: [photon-onboarding] → [photon-onboarding][photon-onboarding-newui]
I can confirm the intended behavior is respected on beta. I verified using Fx 57.0b7 on Windows 10 x64, Ubuntu 14.04 LTS and macOS X 10.12.6.
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.