Closed Bug 1355020 Opened 3 years ago Closed 3 years ago

Include update and other 'notification' UI on hamburger panel in new Photon hamburger panel

Categories

(Firefox :: Toolbars and Customization, enhancement, P1)

53 Branch
enhancement

Tracking

()

VERIFIED FIXED
Firefox 55
Iteration:
55.5 - May 15
Tracking Status
firefox55 --- verified

People

(Reporter: Gijs, Assigned: Gijs)

References

(Blocks 1 open bug)

Details

(Whiteboard: [photon-structure])

Attachments

(3 files)

We currently show update notifications in the hamburger panel, including badges on the panel button. Sideloaded add-on notifications also appear there, and bug 893505 simplified some of the logic there.

When swapping out the panel menu for one with a different layout, these notifications will continue to need a place to go. The badges can stay the same, but the design at https://mozilla.invisionapp.com/share/5ZAEYEW8M#/screens/218570316 will need to be updated to include such notifications.
Whiteboard: [photon]
Depends on: 1355021
Flags: qe-verify?
Priority: -- → P2
Whiteboard: [photon] → [photon-structure]
Flags: qe-verify? → qe-verify+
QA Contact: gwimberly
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Iteration: --- → 55.4 - May 1
Priority: P2 → P1
Iteration: 55.4 - May 1 → 55.5 - May 15
Because of the mix of background-image and list-style-image (for the correctly-positioned image on the left), switching to 1 icon wasn't entirely trivial. There'd also be the question, for the add-on icons, whether we show the add-on icon or the warning icon we show now. So just keeping the status quo seemed simplest. Per discussion on slack, that's what I've done for now.
Comment on attachment 8864187 [details]
Bug 1355020 - part 1: disentangle update notifications from 'PanelUI' and hardcoded IDs,

https://reviewboard.mozilla.org/r/135842/#review138912
Attachment #8864187 - Flags: review?(dothayer) → review+
Comment on attachment 8864200 [details]
Bug 1355020 - part 3: add notification container and styling for photon hamburger panel,

https://reviewboard.mozilla.org/r/135864/#review139596

These changes look a-ok to me! I think it might be handy to explain to Kris the _why_ behind the browser-addons.js changes, which might make it easier for him to dive in and review.

::: browser/themes/shared/customizableui/panelUI.inc.css:678
(Diff revision 2)
>    border-top: 1px solid var(--panel-separator-color);
>    border-bottom: 1px solid transparent;
>    margin-bottom: -1px;
>  }
>  
> +/* in Photon, we have a bottom border as well. Reconcile with the above rule

Yeah, I'm also using 'photon' labeled sections in places... makes them easy to find & merge before we ship 57 - because I think that's when this needs to happen, not after shipping; then we can cleanup things nicely during the 57 cycle.
Attachment #8864200 - Flags: review?(mdeboer) → review+
Comment on attachment 8864188 [details]
Bug 1355020 - part 2: disentangle add-on notifications from 'PanelUI' and their specific spot,

https://reviewboard.mozilla.org/r/135844/#review139786
Attachment #8864188 - Flags: review?(kmaglione+bmo) → review+
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d7331d0d4252
part 1: disentangle update notifications from 'PanelUI' and hardcoded IDs, r=dthayer
https://hg.mozilla.org/integration/autoland/rev/43aad7ae71a6
part 2: disentangle add-on notifications from 'PanelUI' and their specific spot, r=kmag
https://hg.mozilla.org/integration/autoland/rev/b265fdf79862
part 3: add notification container and styling for photon hamburger panel, r=mikedeboer
https://hg.mozilla.org/mozilla-central/rev/d7331d0d4252
https://hg.mozilla.org/mozilla-central/rev/43aad7ae71a6
https://hg.mozilla.org/mozilla-central/rev/b265fdf79862
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
I was able to verify this in Windows and Ubuntu/Linux, but I cannot seem to get any visuals regarding updating Firefox in the hamburger window in the Mac OSX build. Is this intended due to the difference in interface of Mac OSX?
Flags: needinfo?(gijskruitbosch+bugs)
(In reply to Grover Wimberly IV [:Grover-QA] from comment #13)
> I was able to verify this in Windows and Ubuntu/Linux, but I cannot seem to
> get any visuals regarding updating Firefox in the hamburger window in the
> Mac OSX build. Is this intended due to the difference in interface of Mac
> OSX?

Nope, it should work the same way on OS X I think... I don't know off-hand why it wouldn't work on OS X... I've heard my colleagues say they managed to use the new UI, I *think* that was on OS X... Sorry I'm not more help! Not sure how to debug this. I guess you can look, in the build where you're trying to get this to appear, (a) if it works without the photon pref flipped, and (b) if you open view-source:chrome://browser/content/browser.xul, and find in page for "panel-banner-item", does that get a hit? Maybe the nightly for OS X was spun on a build before this bug merged, or something?
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(gwimberly)
Got it, I will try the suggestions from Comment 14 and follow-up.

The way I verified Windows and Ubuntu was from downloading yesterday's Nightly build and flipping the pref and simply waiting for the icon.
Ok, I was able to get a different notification UI element to appear on OSX from testing another feature in conjunction with this one. Looks like everything is good.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Flags: needinfo?(gwimberly)
Depends on: 1364738
Blocks: 1387512
Duplicate of this bug: 767694
You need to log in before you can comment on or make changes to this bug.