carbon-design-system / ibm-products

A Carbon-powered React component library for IBM Products
https://ibm-products.carbondesignsystem.com
Apache License 2.0
93 stars 137 forks source link

User profile images #69

Closed carrenelloyd closed 3 years ago

carrenelloyd commented 4 years ago

Catalog # 27

Link to Design

Maintainers Marion Brülls

Tasks

Working in experimental package

Continuing in experimental package

Review and promote

lee-chase commented 4 years ago

Blocked as the status of the design it was set to 'status: refining' by @carrenelloyd

Invited Marion Brülls to join the conversation here.

Levinson1 commented 4 years ago

Design completed and live

lee-chase commented 4 years ago

Dev thoughts.

  1. Would 'Avatar' be a better name for the component? Calling it UserProfileImage may discourage group use and strictly speaking it may not be an image.
lee-chase commented 4 years ago
  1. IBM Profiles ask the user to provide
    • First name
    • Inital (optional)
    • Last name
    • Display name

Would it not be better to use the first two capitalized letters should be used from the display name.

qbi11y commented 3 years ago

@lee-chase @carrenelloyd who do I need to speak with to come up to speed on this issue?

andrea-island commented 3 years ago

@qbi11y, Marion Brulls and Mitchell Bernstein (wish I could tag them...) own the pattern, but it is currently under discussion with Carmen Darlach for the dark/light theme switching, to determine what the behavior should be for light versus dark theme (map to lighter colors OR a white border around the circle). I've reached out to Carmen and will report back on what the decision is for the pattern. This decision will affect the CSS, but probably not much else.

lee-chase commented 3 years ago

Personally don’t like dark/light theme switches and think we should switch automatically based on theme. Unfortunately, it is only possible to take a best guess at the current Carbon theme based on the value of the background color.

marion-bruells commented 3 years ago

Copying from our slack conversation:

Marion: To your question @lee-chase :

IBM Profiles ask the user to provide
First name
Inital (optional)
Last name
Display name
Would it not be better to use the first two capitalized letters should be used from the display name.

We originally designed it with first and last name – how users also know from box and other applications. I know some applications use also the display name. What benefits are you seeing in using the display name instead on our platforms?

Lee: I felt the need to point out what is asked for elsewhere. If a display name is given I would expect it to be used over the users name.

marion-bruells commented 3 years ago

@lee-chase If display name is elsewhere used over the users name I think we can also follow that path.

qbi11y commented 3 years ago

@marion-bruells @lee-chase @andrea-gm I have run into an issue where the design calls for a size (64px) that Carbon does not support. Is this 64px only for profile images or should it be applied to icons and initials as well?

In regards to font size, the design calls for 8 and 10px. The lowest carbon support is a 12px token. Is there a plan for Carbon to support these lower sizes or should the implementation ignore Carbon token support and force the 8 and 10px sizes?

Responding to the display name conversation. My assumption is the component consumer would pass the entire display name to the component and the component would need to format the display name to fit in the display circle. Is this assumption correct?

lee-chase commented 3 years ago

@qbi11y $spacing-10 and $container-05 are undocumented 64px. $layout-05 is also 64px.

qbi11y commented 3 years ago

@lee-chase @andrea-gm this is ready for an initial engineering review. Please advise on how to proceed.

andrea-island commented 3 years ago

@qbi11y I can do an initial review. I'll schedule some time for us tomorrow (Monday)!

matthewgallo commented 3 years ago

Review for release

Readiness

Engineering review

Standards

Testing

Documentation

qbi11y commented 3 years ago

@marion-bruells can you please review the component from a design perspective: https://ibm-cloud-cognitive.netlify.app/?path=/story/cloud-cognitive-canary-userprofileimage--default

marion-bruells commented 3 years ago

I reviewed in the storybook - overall it looks great. I found some accessibility issues. Here is my feedback:

Text and icon color dark mode

Remove small sizes

Add alt-text

  1. In cases where a text label is in addition to the image in the UI, it doesn’t require this - here the screenreader just reads the text next to it and the user doesn’t require a tooltip on hovering the image either, as the name is placed next to it already. Having alt-text in addition here would just repeat the same meaning - which can be annoying. image

We should consider those two use cases and support this alt-text for 1 - but make sure it can be disabled for case 2.

qbi11y commented 3 years ago

@marion-bruells I removed the small sizes as asked. As far as the text color. Since we are unable to detect the theme, the dev needs to pass the theme into the component. If you didn't update the theme switcher and the theme control on the component would not display correctly in storybook. If you want to see how the component looks on the dark theme, set the theme to 'dark' on the component controls tab and also set it to dark on the theme switcher tab. You should then see the correct colors.

In regards to the tooltip. I see there is conversation about the tooltip component vs browser tooltip. I have currently added the browser tooltip. It's not as visually appealing as the component but it sounds to me it is the best option. If you would like to switch to the component please let me know.

dcwarwick commented 3 years ago

Closed by #758