Skip to content

Ribbon icon not updated for actions using ImageName (VirtualImageList) instead of ImageIndex #132

Description

@joerg-github

Summary

When roAssignImagesFromActionManager is enabled and a TAction's icon is set via ImageName (i.e. via a TVirtualImageList/TImageCollection) instead of the classic ImageIndex, the ribbon command's icon is never updated, neither on initial binding nor on subsequent Action.Update calls.

Only actions that using the ImageIndex property work correctly.

Root cause

Both places that sync an action's icon to the ribbon command hard-code TCustomAction.ImageIndex and never look at ImageName:

1. Lib/UIRibbonActions.pas, TUICommandActionLink.SetImageIndex (line 413):

procedure TUICommandActionLink.SetImageIndex(Value: Integer);
var
  lActionManager: TActionManager;
begin
  inherited;
  if (Value >= 0) and IsImageIndexLinked and (TContainedAction(Self.Action).ActionList is TActionManager) then begin
    lActionManager := TActionManager(TContainedAction(Self.Action).ActionList);
    if Assigned(lActionManager.Images) then
      FClient.SmallImage := TUIImage.Create(lActionManager.Images, Value);
    if Assigned(lActionManager.LargeImages) then
      FClient.LargeImage := TUIImage.Create(lActionManager.LargeImages, Value);
  end;
end;

This is invoked from SetAction/Update with Value = Action.ImageIndex. If an action only ever sets ImageName (leaving ImageIndex at its default, e.g. -1), the Value >= 0 guard fails and the whole body is skipped — the ribbon icon is simply never assigned/updated, on every single update cycle.

2. Lib/UIRibbon.pas, TUIRibbon.ImageListChange (line 1051), which reacts to the action list's image list firing OnChange (e.g. a TVirtualImageList reloading its ImageCollection):

lImageIndex := TCustomAction(lCommand.ActionLink.Action).ImageIndex;

if (lActionManager.Images = Sender) then
  lCommand.SmallImage := TUIImage.Create(Sender as TCustomImageList, lImageIndex)
else if ((lActionManager.LargeImages = Sender)) then
  lCommand.LargeImage := TUIImage.Create(Sender as TCustomImageList, lImageIndex)

Same issue: only ImageIndex is read, ImageName is ignored entirely.

(One possible) Suggested fix

In both locations, prefer ImageName when it is set and resolvable, falling back to ImageIndex otherwise — GetIndexByName is declared right on TCustomImageList (Vcl.ImgList.pas), so this works for both classic and virtual image lists without needing to special-case the list type:

function ResolveActionImageIndex(Action: TCustomAction; Images: TCustomImageList): TImageIndex;
begin
  Result := -1;
  if (Action.ImageName <> '') and Assigned(Images) then
    Result := Images.GetIndexByName(Action.ImageName);
  if Result < 0 then
    Result := Action.ImageIndex;
end;

For older versions of Delphi, the code using ImageName must, of course, be conditional.

It might also makes sense to add the property ImageName to the TRibbonAction<T:TUICommand> class like:

  TRibbonAction<T:TUICommand> = class(TCustomAction)
  private
    fUICommand: T;
  public
    property UICommand: T read fUICommand write fUICommand;
  published
    property Caption;
    property Enabled;
    property HelpContext;
    property HelpKeyword;
    property HelpType;
    property Hint;
    property ImageIndex;
{$IF CompilerVersion >= 35.0}
    property ImageName;
{$IFEND}
    property SecondaryShortCuts;
    property ShortCut default 0;
    property OnExecute;
    property OnHint;
    property OnUpdate;
  end;

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions