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

feat: Use the org acronym when generating the org avatar #1523

Merged
merged 9 commits into from
May 10, 2023

Conversation

tschaffter
Copy link
Member

@tschaffter tschaffter commented May 9, 2023

Changelog

  • Enable the user of the avatar component to specify the value (text) to use instead of generating initials from the name.
  • Limit the length of the value/initials to 1 characters to prevent UI layouts from breaking.
  • Convert the OC avatar component to a standalone component.
  • Update the API client for Angular to get access to the property organization.acronym.

Notes

  • Setting the acronym to avatar.name will result in a single-letter initial (1-word acronym => 1-letter initial). A solution is to set avatar.value to the acronym. However, the avatar.name should not be specified because it takes priority over avatar.value.

Preview

  • The text displayed in the avatar is now limited to 1 characters to prevent UI layout from breaking.
  • The new property organization.acronym can be specified instead of generating initials from organization.name (though this should not affect the generation of 1-letter initials).

Below is what the user would see if these orgs didn't have a logo:

image

Copy link
Contributor

@mdsage1 mdsage1 left a comment

Choose a reason for hiding this comment

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

I like the idea to limit the # of characters displayed in the orgs avatars. I think limit == 2 is a bit too low and would suggest using limit == 3 or limit==4 to avoid end user confusion when viewing the site and prevent overlap in displayed acronyms. A suggestion to avoid this would be to implement a tooltip that will display the full acronym when the user hovers over the avatar.

@vpchung
Copy link
Member

vpchung commented May 10, 2023

Looking over our collected data, upping the max limit to 3 or 4 might not be enough either -- we have some acronyms that are 8 characters long, e.g. EMBL-EBI. This would result in an "acronym" that is either EMB or EMBL, which is the official acronym for "European Molecular Biology Laboratory" (parent organization of EMBL-EBI).

Ergo, I'm arguing for displaying only the first character.

P.S. I like the tooltip idea!

@tschaffter
Copy link
Member Author

@vpchung @mdsage1 Here are some notes from our stand-up meeting:

  • Verena submitted a PR to reduce the font size of the avatar text. This change does not guarantee that the avatar won't break because of long text.
  • The only solution to the above problem is to limit the number of characters that we want to display.
    • Option 1: We limit the number of character to what we can fit inside the circle for the current font size, i.e. 4. Two potential issues: 1) It may be difficult to fit 4 characters if the size of the avatar is smaller, e.g. if we use it in a list (200x200 px could become 40x40 px), 2) a cropped acronym of 4 letters may look weird.
    • Option 2: We limit the length of the text to 1 or 2 characters. Two characters provide more diversity while maintaining good readability.

What is your preferred text length based on the above notes between 1, 2, and 4-letter strings in the avatar?

@tschaffter
Copy link
Member Author

tschaffter commented May 10, 2023

@vpchung @mdsage1 Thanks for the feedback. I prefer the 1-letter initials to the 4-letter. The benefit is that users shouldn't try to make sense out of a 4-letter initials that correspond to a cropped acronym. A 4-letter acronym may also be difficult to read in a smaller version of the avatars, e.g. when displayed in a list (though we could cross that bridge when we are there).

@tschaffter tschaffter merged commit 621bb43 into Sage-Bionetworks:issue-1481 May 10, 2023
@tschaffter tschaffter deleted the use-acronym branch May 10, 2023 19:12
tschaffter added a commit that referenced this pull request May 10, 2023
* Rebase from main

* Rebase from main

* add acronyms; update challenge contributors

* feat: Use the org acronym when generating the org avatar (#1523)

* Update API client for Angular

* Fix mock organizations in app

* Convert the avatar component to standalone

* Enable to specify acronym to the avatar

* Restore src

* Add comment

* Fix test

* Update libs/openchallenges/ui/src/lib/avatar/avatar.component.ts

Co-authored-by: Verena Chung <[email protected]>

* Restore images

---------

Co-authored-by: Verena Chung <[email protected]>

* Get image URL only when key is not null or empty

---------

Co-authored-by: verena <[email protected]>
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.

3 participants