Closed
Bug 989901
Opened 10 years ago
Closed 10 years ago
Change |actionButton.icon['16']| doesn't refresh the icon on toolbar until open a new tab.
Categories
(Add-on SDK Graveyard :: General, defect)
Tracking
(Not tracked)
RESOLVED
INVALID
People
(Reporter: pzhang, Unassigned)
Details
Attachments
(1 file)
9.86 KB,
application/x-xpinstall
|
Details |
Here is the main.js: let state = 'offline'; function getIcon() { return require('sdk/self').data.url(state + '.png') } let button = require('sdk/ui/button/action').ActionButton({ id: 'myid', label: 'mybutton', icon: { '16': getIcon(), '32': getIcon() }, onClick: function() { state = state == 'offline' ? 'online' : 'offline'; button.icon['16'] = getIcon(); } });
Reporter | ||
Comment 1•10 years ago
|
||
BTW, set icon with an url instead of an object works.
Comment 2•10 years ago
|
||
(In reply to Pin Zhang [:pzhang] from comment #0) > function getIcon() { > return require('sdk/self').data.url(state + '.png') > } You don't need the `data.url` thingy in the new API. You can just have: const getIcon = () => './' + state + '.png'; Any path that starts with './' is recognized as data folder path; see the examples: https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/ui_button_action > onClick: function() { > state = state == 'offline' ? 'online' : 'offline'; > button.icon['16'] = getIcon(); > } It's not supposed to work; if you want to update the icons you need to update the whole set of icon, so: button.icon = {'16': getIcon(), '32': getIcon() }; However, I would probably suggest something like that: let isOnline = false; let onlineIcons = { 'true': { '16': './online16.png', '32': './online32.png' }, 'false': { '16': './offline16.png', '32': './offline32.png' } } And in the `onClick`: onClick: function() { isOnline = !isOnline; this.icon = onlineIcons[isOnline]; } Also, for such use case, you could use `ToggleButton` instead of `ActionButton`, see: https://developer.mozilla.org/en-US/Add-ons/SDK/Low-Level_APIs/ui_button_toggle But notice that the checked state is by default per window, if you want to have it globally you have to some code to remove the checked state per window and add it globally. So it would looks like: let button = ToggleButton({ id: 'my-button', icon: './foo16.png', label: 'my button', onClick: function(state) { this.icon = icons[state.checked]; // set the icon this.state('window', null); // remove the window's state this.checked = state.checked; // set the global state } }); In that case you will get the "pressed" visual state and also the `checked` property. Ah, and of course if your icons points to the same png for all resolution, you don't need that, you can just use the url instead. We should probably freeze the `button.icon` object – actually, I thought we already did, but maybe apparently not in the constructor – to avoid future misleading, like we have done for `panel.position`.
Comment 3•10 years ago
|
||
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #2) > We should probably freeze the `button.icon` object – actually, I thought we > already did, but maybe apparently not in the constructor – to avoid future > misleading, like we have done for `panel.position`. I filed Bug 990023 to fix the reference / frozen thing. It seems also that there is a regression on `panel.position` too (Bug 990026).
Comment 4•10 years ago
|
||
Due the bug 990023 landed, and the explanation given, I will mark this as INVALID.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
Reporter | ||
Comment 5•10 years ago
|
||
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #3) > (In reply to Matteo Ferretti [:matteo] [:zer0] from comment #2) > > > We should probably freeze the `button.icon` object – actually, I thought we > > already did, but maybe apparently not in the constructor – to avoid future > > misleading, like we have done for `panel.position`. > > I filed Bug 990023 to fix the reference / frozen thing. It seems also that > there is a regression on `panel.position` too (Bug 990026). Froze icon object make sence, thanks.
You need to log in
before you can comment on or make changes to this bug.
Description
•