Closed Bug 951921 Opened 11 years ago Closed 11 years ago

Document the new ActionButton and ToggleButton API

Categories

(Add-on SDK Graveyard :: Documentation, defect, P1)

x86
macOS
defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zer0, Assigned: wbamberg)

References

Details

Attachments

(1 file)

We need to document the new Button API exposed by bug 951317
Assignee: nobody → zer0
Depends on: 951317
Blocks: 958454
Attached file mdn.html (deleted) —
This is just for the action button. I suppose that for the toggle button we should duplicate this and just add the bit for "checked" and "change"?
Assignee: zer0 → wbamberg
Attachment #8361946 - Flags: review?(zer0)
(In reply to Will Bamberg [:wbamberg] from comment #1) > Created attachment 8361946 [details] > mdn.html > > This is just for the action button. I suppose that for the toggle button we > should duplicate this and just add the bit for "checked" and "change"? You suppose well. We'll probably add some sugar thingy in the future, but at the moment that's it. Also, I was wondering if you think the `onchange` event is just an overkill: I added because we have it in HTML/XUL checkbox's type of input, fired after `onclick`, but like in HTML in this specific case everything could be done by `click` event. About the review, looks perfect! I would just maybe modify the example about using `state` with the button's instance itself, so instead passing just one property: button.state(button, { "label" : "button-global label" }); I would set more than one, like `label` and `icon` for instance. So that it's explicitly clear what is the use case of passing `button`: instead of call every single property (button.label = ..., button.icon = ...) we could pass a set of property (e.g. like jquery) and devs can also keep this set as object (e.g. in `ToggleButton` having a `checkedStatus` object, that can be used for global or window / tab state) could be more useful. I would also emphasize that the `state` passed by `click` and the `state` obtained by the namesake method, is a "snapshot": it means is an object with the button's property at the time where the button was clicked or the state was asked: to be clear, is not a living object: changing the button's property won't update the `state`: button.label = 'foo'; let state = button.state(button); button.label = 'bar'; console.log(state.label) // foo That is made on purpose, in order to avoid racing issues where two part of code trying to update the same properties where another try to elaborate the state for example. The documentation is already awesome, so I r+. If you agreed on those changes, add them too.
Attachment #8361946 - Flags: review?(zer0) → review+
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #3) > (In reply to Will Bamberg [:wbamberg] from comment #1) > > Created attachment 8361946 [details] > > mdn.html > > > > This is just for the action button. I suppose that for the toggle button we > > should duplicate this and just add the bit for "checked" and "change"? > > You suppose well. We'll probably add some sugar thingy in the future, but at > the moment that's it. > Also, I was wondering if you think the `onchange` event is just an overkill: > I added because we have it in HTML/XUL checkbox's type of input, fired after > `onclick`, but like in HTML in this specific case everything could be done > by `click` event. I think it's fine to have "change" as well, if that's what developers are used to seeing. > About the review, looks perfect! I would just maybe modify the example about > using `state` with the button's instance itself, so instead passing just one > property: > > button.state(button, { > "label" : "button-global label" > }); > > I would set more than one, like `label` and `icon` for instance. So that > it's explicitly clear what is the use case of passing `button`: instead of > call every single property (button.label = ..., button.icon = ...) we could > pass a set of property (e.g. like jquery) and devs can also keep this set as > object (e.g. in `ToggleButton` having a `checkedStatus` object, that can be > used for global or window / tab state) could be more useful. That makes sense to me. I did wonder what it was for. Thanks Matteo!
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: