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)

x86_64
Linux
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED INVALID

People

(Reporter: pzhang, Unassigned)

Details

Attachments

(1 file)

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();
    }
  });
BTW, set icon with an url instead of an object works.
(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`.
(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).
Due the bug 990023 landed, and the explanation given, I will mark this as INVALID.
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → INVALID
(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.

Attachment

General

Created:
Updated:
Size: