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

dev/core#1899 specify display mode for action links with icons #17947

Merged
merged 1 commit into from
Jul 25, 2020

Conversation

agh1
Copy link
Contributor

@agh1 agh1 commented Jul 24, 2020

This is a 5.28 RC equivalent of #17933.

Overview

Changes in #17319 allow action links to be specified with icon to display the icon only instead of the link text. However, case activity action links have icons specified dating back to #14349 because they serve double-duty as specifying the buttons when viewing a single activity. This is a nice pattern to follow elsewhere in the future, but the result is that it forced the action links to render as icons only.

This allows the $iconMode to be set in CRM_Core_Action::formLink() in order to decide whether to display each action link as an icon (if specified), text, or both.

Before

Activity listing when viewing a case:
image

After

image

Technical Details

I set it so icon mode doesn't stash extra items under the "more >" both since there's more real estate and it just looks stupid. The screenshot below is what the same activity listing would appear as if it were in icon mode now (though it is not set this way as submitted):

image

Note that icon mode displays the text if there is no icon specified.

Comments

The only place where the links are displayed as icons is the edit payment link (a pencil) when viewing a contribution.

@civibot
Copy link

civibot bot commented Jul 24, 2020

(Standard links)

@eileenmcnaughton
Copy link
Contributor

This was reviewed against master - MOP

We should also port to 5.27

@eileenmcnaughton
Copy link
Contributor

unrelated fail

@eileenmcnaughton
Copy link
Contributor

Added Ok without test as we are less strict on this when we ask someone to fix a regression under time pressure

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

Successfully merging this pull request may close these issues.

2 participants