Closed
Bug 1247724
Opened 8 years ago
Closed 8 years ago
[FlyWeb] Update popover UI to conform to existing styles
Categories
(Core :: Networking, defect)
Core
Networking
Tracking
()
RESOLVED
FIXED
People
(Reporter: justindarc, Assigned: justindarc)
References
Details
Attachments
(1 file)
7.06 KB,
patch
|
djvj
:
review+
|
Details | Diff | Splinter Review |
The current popover UI for FlyWeb is just a rough draft for getting things working. The UI should conform to the existing look & feel of the rest of similar toolbars in Firefox Desktop.
Assignee | ||
Comment 1•8 years ago
|
||
I opened this bug to track landing of a patch I already built. Assigning to myself.
Assignee | ||
Comment 2•8 years ago
|
||
Attachment #8720991 -
Flags: review?(kvijayan)
Comment 3•8 years ago
|
||
Comment on attachment 8720991 [details] [diff] [review] bug1247724.patch Review of attachment 8720991 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. Just minor nits in review comments. R+. ::: browser/components/customizableui/CustomizableWidgets.jsm @@ +1201,5 @@ > this.discoveryManager.connectToService(serviceId, { > + connectSucceeded(connectedService) { > + let {hostname} = connectedService; > + let url = "http://" + hostname + "/"; > + log.debug("CONNECT TO: ", url); Nit: Cleaner log message here would be nice. e.g.: ("FlyWeb connecting to: ", url) ::: browser/components/customizableui/content/panelUI.inc.xul @@ +313,5 @@ > <button id="PanelUI-panic-view-button" > label="&panicButton.view.forgetButton;"/> > </vbox> > </panelview> > Nit: not your problem, but it's right next to your code. Whitespace on blank line.
Attachment #8720991 -
Flags: review?(kvijayan) → review+
Assignee | ||
Comment 4•8 years ago
|
||
https://hg.mozilla.org/projects/larch/rev/bc7fdd4df1f68f367ffb887a12e5fe54bc2c91ed Bug 1247724 - [FlyWeb] Update popover UI to conform to existing styles;r=djvj
Assignee | ||
Comment 5•8 years ago
|
||
Landed on larch: http://hg.mozilla.org/projects/larch/rev/bc7fdd4df1f6
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•