Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Android] Icon downloaded and displayed for overflow Action #5809

Merged

Conversation

karthikgopal36
Copy link
Contributor

@karthikgopal36 karthikgopal36 commented May 11, 2021

To display icon on a dropdown item, we usually fetch it from rendered action element and this works only when the dropdown item added to window. Otherwise, icon will not be displayed.
In this PR, we downloaded icon for dropdown item separately instead of fetching it from rendered action element, so the icon can be fetched and displayed on any condition.

Also, rootLevelActions flag added to the onDisplayOverflowActionMenu() callback to distinguish between Action & ActionSet

Microsoft Reviewers: Open in CodeFlow

rootLevelActions added to onDisplayMenuAction callback.
Copy link
Member

@golddove golddove left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some suggestions

* @param view Overflow action view.
* @param menuItemList list of view of rendered secondary action elements.
* @param view Overflow action view.
* @param isRootLevelActions indicates action is part of root level actions or action set elements in body.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param isRootLevelActions indicates action is part of root level actions or action set elements in body.
* @param isRootLevelActions true if action is defined at root level, false if part of ActionSet

Comment on lines 89 to 106
if (!iconUrl.isEmpty())
{
dropDownItem.post(() ->
IconPlacement iconPlacement = hostConfig.GetActions().getIconPlacement();
if (!renderArgs.getAllowAboveTitleIconPlacement())
{
Drawable[] drawables = button.getCompoundDrawablesRelative();
dropDownItem.setCompoundDrawablePadding(button.getCompoundDrawablePadding());
dropDownItem.setCompoundDrawablesRelativeWithIntrinsicBounds(drawables[0], drawables[1], drawables[2], drawables[3]);
});
iconPlacement = IconPlacement.LeftOfTitle;
}

ActionElementRendererIconImageLoaderAsync imageLoader = new ActionElementRendererIconImageLoaderAsync(
renderedCard,
dropDownItem,
hostConfig.GetImageBaseUrl(),
iconPlacement,
hostConfig.GetActions().getIconSize(),
hostConfig.GetSpacing().getDefaultSpacing(),
context
);
imageLoader.executeOnExecutor(AsyncTask.THREAD_POOL_EXECUTOR, iconUrl);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this is identical to code in ActionElementRenderer. Consider moving it into a helper within ActionElementRenderer and calling it, instead of duplicating.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

moved to helper class.

@golddove golddove changed the title Icon downloaded and displayed for DropdownItem. [Android] Icon downloaded and displayed for overflow Action May 11, 2021
@karthikgopal36 karthikgopal36 merged commit 8d84719 into main May 11, 2021
@karthikgopal36 karthikgopal36 deleted the bug/ActionOverflow/fix_overflow_dropdown_icon_issue branch May 11, 2021 22:02
michaelfarnsworth pushed a commit to michaelfarnsworth/AdaptiveCards that referenced this pull request Nov 10, 2022
…t#5809)

* Icon downloaded and displayed for DropdownItem.
rootLevelActions added to onDisplayMenuAction callback.

* Image loading functionality moved to helper class.
rankush pushed a commit to rankush/AdaptiveCards that referenced this pull request May 8, 2024
…t#5809)

* Icon downloaded and displayed for DropdownItem.
rootLevelActions added to onDisplayMenuAction callback.

* Image loading functionality moved to helper class.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants