Closed
Bug 585390
Opened 14 years ago
Closed 14 years ago
add panel property to Widget API
Categories
(Add-on SDK Graveyard :: General, enhancement)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
0.7
People
(Reporter: myk, Assigned: myk)
References
Details
Attachments
(1 file)
|
(deleted),
patch
|
dietrich
:
review+
|
Details | Diff | Splinter Review |
The Widget API should have a panel property that simplifies the common case of a widget that displays a panel when you click on it.
This bug is for the "panel property of Widget API" portion of the all-in-one patch in bug 494238, which I'm breaking into smaller pieces to make it easier to review.
Attachment #463869 -
Flags: review?(dietrich)
|
||
Comment 1•14 years ago
|
||
Comment on attachment 463869 [details] [diff] [review]
patch v1: implements enhancement
>+ <td><tt>panel</tt></td>
>+ <td>
>+ A Panel to open when the user clicks on the widget. See the `panel`
>+ module for more information about the `Panel` objects to which this
>+ option can be set and the `reddit-panel` example add-on for an example
>+ of using this option.
>+
>+ Note: if you also specify an `onClick` callback function, it will be
>+ called instead of the panel being opened.
>+ </td>
>+ </tr>
maybe add a note explaining that it's still possible to open the panel manually if you have a custom click handler.
>+ panel: {
>+ is: ["null", "undefined", "object"],
>+ },
should check that it's actually a panel object.
>@@ -197,16 +206,18 @@ let browserManager = {
> // all currently registered windows, and when new windows are registered it
> // will be added to them, too.
> addItem: function browserManager_addItem(item) {
> let idx = this.items.indexOf(item);
> if (idx > -1)
> throw new Error("The widget " + item + " has already been added.");
> this.items.push(item);
> this.windows.forEach(function (w) w.addItems([item]));
>+ if (item.panel)
>+ panels.add(item.panel);
will the panel module add panels to each open window? if so, this could be done just once, in the widget ctor.
(hm, i should've reviewed the panels core patch first!)
>@@ -217,16 +228,18 @@ let browserManager = {
> // of all windows that are currently registered.
> removeItem: function browserManager_removeItem(item) {
> let idx = this.items.indexOf(item);
> if (idx == -1) {
> throw new Error("The widget " + item + " has not been added " +
> "and therefore cannot be removed.");
> }
> this.items.splice(idx, 1);
>+ if (item.panel)
>+ panels.remove(item.panel);
similar to the previous comment - if panel handles all windows, then this should move to the widget dtor.
>+exports.testPanelWidget = function testPanelWidget(test) {
>+ let widgets = require("widget");
>+ let widget = widgets.Widget({
>+ label: "panel widget",
>+ content: "<div id='me'>foo</div>",
>+ onLoad: function(e) {
>+ sendMouseEvent({type:"click"}, "me", e.target.defaultView);
>+ },
>+ panel: require("panel").Panel({
>+ content: "data:text/html,<body>Look ma, a panel!</body>",
>+ onShow: function() {
>+ widgets.remove(widget);
>+ test.pass("panel displayed on click");
>+ test.done();
>+ }
>+ })
>+ });
>+ widgets.add(widget);
>+ test.waitUntilDone();
>+};
should add a test for when there's both a panel and a click handler defined.
r=me with comments addressed.
Attachment #463869 -
Flags: review?(dietrich) → review+
|
Assignee | |
Comment 2•14 years ago
|
||
Note: I pulled the changes for this back into the patch for bug 494238, whose example depends on it. So once that bug is resolved, this one can be too.
|
|
Assignee | |
Updated•14 years ago
|
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
|
|
Assignee | |
Comment 3•14 years ago
|
||
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.
To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
You need to log in
before you can comment on or make changes to this bug.
Description
•