Closed Bug 1171219 Opened 4 years ago Closed 4 years ago

Support for badged toolbarbuttons with panels

Categories

(Firefox :: Toolbars and Customization, defect)

34 Branch
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 41
Tracking Status
firefox41 --- fixed

People

(Reporter: billm, Assigned: billm)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch patchSplinter Review
I have a problem where a toolbarbutton with type="panel" isn't allowed to include a badge. This patch seems to fix the problem.
Attachment #8614931 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8614931 [details] [diff] [review]
patch

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

Neil, does this look right to you? I kind of wish we could do this without yet another binding.

Bill, is there some reason you didn't keep the hbox/vbox structure inside the toolbarbutton[type="menu"]'s structure? Also, have you tested what this looks like in practice, ie if the styling all works out?
Attachment #8614931 - Flags: review?(gijskruitbosch+bugs) → review?(neil)
Comment on attachment 8614931 [details] [diff] [review]
patch

Seems reasonable to me, but I'd add support for [type="menu"] too for consistency, unless there's a particular reason not to.
Attachment #8614931 - Flags: review?(neil) → review+
(In reply to Gijs Kruitbosch from comment #1)
> Neil, does this look right to you? I kind of wish we could do this without
> yet another binding.
I guess the way to go there would be to wrap the <img> in a box in the base binding, then when you want a badge, add a binding to the wrapper box that provides the badge.
https://hg.mozilla.org/mozilla-central/rev/fd26eba375ae
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 41
You need to log in before you can comment on or make changes to this bug.