[Avatar] Remove customer images #2453

kyledurand · 2019-11-21T16:28:18Z

WHY are these changes introduced?

Fixes part of: https://github.com/Shopify/polaris-ux/issues/333
Fixes: https://github.com/Shopify/polaris-ux/issues/92

Before	After

WHAT is this pull request doing?

Customer avatars are mostly unhelpful and sometimes straight up confusing. This PR replaces the multiple images with one default gray version.

How to 🎩

🖥 Local development instructions
🗒 General tophatting guidelines
📄 Changelog guidelines

Copy-paste this code in playground/Playground.tsx:

import React from 'react';
import {
  ResourceList,
  ResourceItem,
  Card,
  Page,
  Avatar,
  Stack,
  TextStyle,
} from '../src';

export function Playground() {
  return (
    <Page title="Playground">
      <Card>
        <Card.Section>
          <Stack alignment="baseline">
            <Avatar initials="KD" name="d" />
            <Avatar initials="KD" name="e" />
            <Avatar initials="KD" name="f" />
            <Avatar initials="KD" name="g" />
            <Avatar initials="KD" name="h" />
            <div
              style={{borderLeft: '1px solid hsl(0, 0%, 50%)', height: '40px'}}
            />
            <Avatar customer initials="KD" name="e" />
            <Avatar
              initials="JD"
              name="John D"
              source="https://images.unsplash.com/photo-1574157573248-293553e90ead?ixlib=rb-1.2.1&ixid=eyJhcHBfaWQiOjEyMDd9&auto=format&fit=crop&w=1534&q=80"
            />
          </Stack>
        </Card.Section>
        <Card.Section>
          <Card>
            <ResourceList
              resourceName={{singular: 'customer', plural: 'customers'}}
              items={[
                {
                  id: 257,
                  url: 'customers/256',
                  name: 'Carl Winslow',
                  location: 'Los Angeles, USA',
                },
                {
                  id: 258,
                  url: 'customers/256',
                  name: 'Harriette Winslow',
                  location: 'Los Angeles, USA',
                },
                {
                  id: 259,
                  url: 'customers/256',
                  name: 'Edward "Eddie" Winslow',
                  location: 'Los Angeles, USA',
                },
                {
                  id: 260,
                  url: 'customers/256',
                  name: 'Laura Lee Winslow',
                  location: 'Los Angeles, USA',
                },
                {
                  id: 261,
                  url: 'customers/256',
                  name: 'Judith "Judy" Winslow',
                  location: 'Los Angeles, USA',
                },
                {
                  id: 262,
                  url: 'customers/256',
                  name: 'Steven "Steve" Quincy Urkel',
                  location: 'Los Angeles, USA',
                },
              ]}
              renderItem={(item) => {
                const {id, url, name, location} = item;
                const media = <Avatar customer size="medium" name={name} />;
                return (
                  <ResourceItem
                    id={id}
                    url={url}
                    media={media}
                    accessibilityLabel={`View details for ${name}`}
                  >
                    <h3>
                      <TextStyle variation="strong">{name}</TextStyle>
                    </h3>
                    <div>{location}</div>
                  </ResourceItem>
                );
              }}
            />
          </Card>
        </Card.Section>
      </Card>
    </Page>
  );
}

🎩 checklist

Tested on mobile
Tested on multiple browsers
Tested for accessibility
Updated the component's README.md with documentation changes
Tophatted documentation changes in the style guide
For visual design changes, pinged one of @ HYPD, @ mirualves, @ sarahill, or @ ry5n to update the Polaris UI kit

github-actions · 2019-11-21T16:30:09Z

🟡 This pull request modifies 14 files and might impact 13 other files. This is an average splash zone for a change, remember to tophat areas that could be affected.

Details:

All files potentially affected (total: 13)

📄 UNRELEASED.md (total: 0)

Files potentially affected (total: 0)

🎨 src/components/Avatar/Avatar.scss (total: 13)

Files potentially affected (total: 13)

🧩 src/components/Avatar/Avatar.tsx (total: 12)

Files potentially affected (total: 12)

📄 src/components/Avatar/images/avatar-1.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-2.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-3.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-4.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-5.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-6.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-7.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-8.svg (total: 0)

Files potentially affected (total: 0)

📄 src/components/Avatar/images/avatar-9.svg (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Avatar/images/index.ts (total: 0)

Files potentially affected (total: 0)

🧩 src/components/Avatar/tests/Avatar.test.tsx (total: 0)

Files potentially affected (total: 0)

kyledurand · 2019-11-21T16:32:32Z

@sarahill I'm in no rush to get this merged but I think we'll have to update the UI kit when it does

sarahill

This is looking good! Noticed a couple things:

Hover states
When you hover on a list item the avatars get pretty lost.

Suggestion: Add a 2px white border to help it stand out a little more.

High contrast mode
In windows high contrast mode the background of the avatars isn't visible.

Suggestion: Add a border to give these a container.

Before	After

chloerice

💯on adding a border for contrast. Code-wise this looks good to me 🚀

kyledurand · 2020-01-20T21:12:01Z

src/components/Avatar/Avatar.scss


+  @media (-ms-high-contrast: active) {
+    border: 1px solid ms-high-contrast-color('text');
+  }


@sarahill I've finally been able to circle back to this and add a border for high contrast mode:

src/components/Avatar/Avatar.tsx

sarahill

Looks great! Thanks, Kyle!

kaelig · 2020-01-28T03:25:05Z

Hey Kyle, does this call for a style guide update? (Avatars spotted in https://polaris.shopify.com/design/illustrations#section-partner-resources)

kyledurand · 2020-01-28T03:55:13Z

does this call for a style guide update?

I believe so! I've asked Sara to update the UI kit but if the styleguide won't be updated automatically then we'll have to do it there as well. Want to pair on that @kaelig?

sarahill · 2020-01-28T13:10:35Z

We're holding off on any updates to the existing UI Kit since we're actively working on the new kit now.

…

On Mon, Jan 27, 2020 at 10:55 PM Kyle Durand ***@***.***> wrote: does this call for a style guide update? I believe so! I've asked Sara to update the UI kit but if the styleguide won't be updated automatically then we'll have to do it there as well. Want to pair on that @kaelig <https://github.com/kaelig>? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2453?email_source=notifications&email_token=ACB2XNOB7OHAJVBAKRCYUO3Q76UCFA5CNFSM4JQE5KR2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKB547Q#issuecomment-579067518>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACB2XNJT27PLJZOJTWDTKLLQ76UCFANCNFSM4JQE5KRQ> .

kaelig · 2020-02-05T05:10:37Z

src/components/Avatar/Avatar.tsx

-      </span>
-    ) : null;
+  const avatarBody = customer ? (
+    <g fill="currentColor" fillRule="nonzero">


Hey @kyledurand – as I was looking at the component, I noticed this minor refactor opportunity:

- <g fill="currentColor" fillRule="nonzero"> - <path d="M8.28 27.5A14.95 14.95 0 0120 21.8c4.76 0 8.97 2.24 11.72 5.7a14.02 14.02 0 01-8.25 5.91 14.82 14.82 0 01-6.94 0 14.02 14.02 0 01-8.25-5.9zM13.99 12.78a6.02 6.02 0 1112.03 0 6.02 6.02 0 01-12.03 0z" /> - </g> + <path fill="currentColor" d="M8.28 27.5A14.95 14.95 0 0120 21.8c4.76 0 8.97 2.24 11.72 5.7a14.02 14.02 0 01-8.25 5.91 14.82 14.82 0 01-6.94 0 14.02 14.02 0 01-8.25-5.9zM13.99 12.78a6.02 6.02 0 1112.03 0 6.02 6.02 0 01-12.03 0z" />

Not a huge win, but I thought I'd bring it up in case you go back to this at some point!

Nice find! I have another Avatar PR up so I just made the change there: 72f8997

kyledurand force-pushed the avatar_remove-customer-images branch from 63ef11d to f209e5e Compare November 21, 2019 16:30

kyledurand requested review from BPScott, chloerice and sarahill November 21, 2019 16:31

sarahill requested changes Nov 22, 2019

View reviewed changes

chloerice approved these changes Nov 25, 2019

View reviewed changes

kyledurand force-pushed the avatar_remove-customer-images branch from f209e5e to a51e76b Compare January 20, 2020 21:10

kyledurand commented Jan 20, 2020

View reviewed changes

kyledurand force-pushed the avatar_remove-customer-images branch from a51e76b to 413934d Compare January 20, 2020 21:18

BPScott reviewed Jan 21, 2020

View reviewed changes

src/components/Avatar/Avatar.tsx Show resolved Hide resolved

src/components/Avatar/Avatar.tsx Outdated Show resolved Hide resolved

kyledurand force-pushed the avatar_remove-customer-images branch from 413934d to efbd5cd Compare January 21, 2020 15:29

kyledurand changed the title ~~[Avavtar] Remove customer images~~ [Avatar] Remove customer images Jan 21, 2020

sarahill self-requested a review January 24, 2020 20:45

sarahill approved these changes Jan 24, 2020

View reviewed changes

kyledurand force-pushed the avatar_remove-customer-images branch from efbd5cd to ca648e7 Compare January 27, 2020 21:10

Remove customer images from avatar

246ac8a

kyledurand force-pushed the avatar_remove-customer-images branch from ca648e7 to 16212ae Compare January 27, 2020 22:01

Merging color work

b9ba7fe

kyledurand force-pushed the avatar_remove-customer-images branch from 16212ae to b9ba7fe Compare January 27, 2020 22:01

Merge branch 'master' into avatar_remove-customer-images

a2bc66a

kyledurand merged commit bb917b7 into master Jan 28, 2020

kyledurand deleted the avatar_remove-customer-images branch January 28, 2020 18:33

kaelig reviewed Feb 5, 2020

View reviewed changes

[Avatar] Remove customer images #2453

[Avatar] Remove customer images #2453

Uh oh!

Conversation

kyledurand commented Nov 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

WHY are these changes introduced?

WHAT is this pull request doing?

How to 🎩

🎩 checklist

Uh oh!

github-actions bot commented Nov 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Files potentially affected (total: 0)

Files potentially affected (total: 13)

Files potentially affected (total: 12)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Files potentially affected (total: 0)

Uh oh!

kyledurand commented Nov 21, 2019

Uh oh!

sarahill left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chloerice left a comment

Choose a reason for hiding this comment

Uh oh!

kyledurand Jan 20, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sarahill left a comment

Choose a reason for hiding this comment

Uh oh!

kaelig commented Jan 28, 2020

Uh oh!

kyledurand commented Jan 28, 2020

Uh oh!

sarahill commented Jan 28, 2020 via email

Uh oh!

kaelig Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

kyledurand Feb 5, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kyledurand commented Nov 21, 2019 •

edited

Loading

github-actions bot commented Nov 21, 2019 •

edited

Loading

sarahill left a comment •

edited

Loading