[terra-avatar] Changed Shared user variant to Generic Avatar variant #2620

supreethmr · 2019-08-29T18:29:38Z

Summary

Resolves #2026

Additional Details

Replaced Shared user variant with Generic Variant and added sub-variants single-user, shared-user, provider variants for Generic variant.

nramamurth · 2019-08-30T04:58:04Z

packages/terra-avatar/src/variants/Generic.jsx

+    genericIconClassNames = cx(['icon', GENERIC_VARIANTS.SHARED_USER]);
+  } else if (variant === GENERIC_VARIANTS.PROVIDER) {
+    genericIconClassNames = cx(['icon', GENERIC_VARIANTS.PROVIDER]);
+  }

  /* eslint-disable react/forbid-dom-props */
  return (
    <div {...attributes} className={multiUserClassNames} style={customStyles}>


I'd rename multiUserClassNames.

updated here

nramamurth · 2019-08-30T05:33:16Z

packages/terra-avatar/CHANGELOG.md

@@ -3,6 +3,8 @@ ChangeLog

 Unreleased
 ----------
+### Changed


This should be a MVB, right? Removing a variant altogether is a breaking change.

yes this will be MVB..

nramamurth · 2019-08-30T09:49:25Z

packages/terra-avatar/docs/upgrade-guide.md

+
+### Added
+* `Generic` Variant with props same as `Shared User` and with `Variant` prop to support sub-variants like `single-user`, `shared-user` and `provider`.
+* `variant` prop. this will take values for sub-variants `single-user`, `shared-user` and `provider`.


This is confusing to me as I understand that you are probably talking about the variant prop of Generic sub-component but at first glance, I felt this is in contradiction to what was mentioned earlier.

I'd remove this line altogether.

Replaced line with suggested line below..!

nramamurth · 2019-08-30T09:53:44Z

packages/terra-avatar/docs/upgrade-guide.md

+* `Shared User` Variant From Avatar Variants
+
+### Added
+* `Generic` Variant with props same as `Shared User` and with `Variant` prop to support sub-variants like `single-user`, `shared-user` and `provider`.


Suggested change

* `Generic` Variant with props same as `Shared User` and with `Variant` prop to support sub-variants like `single-user`, `shared-user` and `provider`.

* the `generic` subcomponent that replaces the `sharedUser` subcomponent with a new `variant` prop that can be `single-user`, `shared-user`, or `provider`.

Thank you @nramamurth. yes that sounds more appropriate. updated here : 5f17135

ShettyAkarsh · 2019-08-30T12:28:25Z

packages/terra-avatar/src/variants/Generic.jsx

-    <div {...attributes} className={multiUserClassNames} style={customStyles}>
-      {multiUserContent}
+    <div {...attributes} className={GenericUserClassNames} style={customStyles}>
+      <span className={genericIconClassNames} role="img" aria-label={alt} alt={alt} aria-hidden={isAriaHidden} />


do we need aria-hidden attribute also here ?

ok got it, this is actually a prop

ShettyAkarsh · 2019-08-30T12:48:24Z

packages/terra-avatar/src/variants/Generic.jsx


 const cx = classNames.bind(styles);

+const GENERIC_VARIANTS = {


@supreethmr we will have to export this separately so that consumer can access it.

Added export : 4a470cf

yuderekyu

We have identical single, multi, and provider examples. Let's eliminate some redundancy.

Want to add an example that toggles between the three generic variants? Then, remove all multi and provider examples.

packages/terra-avatar/docs/upgrade-guide.md

jeremyfuksa · 2019-10-10T15:46:41Z

UX Input
In the standard Avatar variant, it was discussed that we wouldn't show the fallback icon because that variant is only for initials. That would also mean that the deceased example would show initials and not an icon.

ryanthemanuel · 2019-10-24T18:19:51Z

packages/terra-avatar/CHANGELOG.md

@@ -3,6 +3,9 @@ ChangeLog

 Unreleased
 ----------
+### Breaking Changes
+* `generic` subcomponent replaces the `sharedUser` subcomponent


Can we add a rationale for this breaking change? We're trying to make sure we provide that for breaking changes going forward.

I have modified the change log for this code changes: 128dd37 . Let me know if any changes needs to be made.

jeremyfuksa

Tested: PR #2620
Resolves: Issue #2026

Reviewed:

Follows pattern considerations documented in the current Cerner Design Standards: Avatar - User Experience - Cerner Wiki
Follows UX defined design implementation for Avatar
Follows defined theme designs: terra-theme

Verified by @jeremyfuksa, Marking as Verified, Ready for Release.

Pass
UX Review

…iant # Conflicts: # packages/terra-avatar/docs/generic.md # packages/terra-avatar/package.json # packages/terra-avatar/src/terra-dev-site/doc/avatar/About.1.doc.mdx

Changed Shared user to Generic Avatar Varaint

aac0063

supreethmr requested review from bjankord, mjhenkes, ryanthemanuel, tbiethman and yuderekyu as code owners August 29, 2019 18:29

bjankord temporarily deployed to terra-core-deployed-pr-2620 August 29, 2019 18:30 Inactive

supreethmr requested review from neilpfeiffer, ShettyAkarsh and jeremyfuksa August 29, 2019 18:30

supreethmr self-assigned this Aug 29, 2019

supreethmr added the 📦 terra-avatar label Aug 29, 2019

supreethmr requested a review from nramamurth August 30, 2019 04:29

nramamurth reviewed Aug 30, 2019

View reviewed changes

supreethmr added Major Version Bump UX Review Required labels Aug 30, 2019

Added upgrade Guide for Breaking Changes

c7d376e

bjankord temporarily deployed to terra-core-deployed-pr-2620 August 30, 2019 09:35 Inactive

nramamurth reviewed Aug 30, 2019

View reviewed changes

Upgrade Guide Corrections

5f17135

bjankord temporarily deployed to terra-core-deployed-pr-2620 August 30, 2019 10:40 Inactive

ShettyAkarsh reviewed Aug 30, 2019

View reviewed changes

bjankord approved these changes Aug 30, 2019

View reviewed changes

export added for constants

4a470cf

bjankord temporarily deployed to terra-core-deployed-pr-2620 September 3, 2019 05:35 Inactive

yuderekyu reviewed Sep 4, 2019

View reviewed changes

packages/terra-avatar/docs/upgrade-guide.md Outdated Show resolved Hide resolved

mergeconflicts

07b365e

bjankord temporarily deployed to terra-core-deployed-pr-2620 October 10, 2019 06:18 Inactive

jeremyfuksa removed the 🚧 Blocked: UX Input label Oct 10, 2019

nramamurth mentioned this pull request Oct 15, 2019

[terra-hyperlink] Switched themeable icons provided by CSS to imported terra-icon component. #2666

Merged

Merge branch 'master' into Avatar-Provider-Variant

bee5afb

bjankord temporarily deployed to terra-core-deployed-pr-2620 October 16, 2019 08:40 Inactive

removed theme variables update from upgrade guide

c03aa55

bjankord temporarily deployed to terra-core-deployed-pr-2620 October 16, 2019 15:24 Inactive

Added user Icon example in upgrade guide

5d5682b

bjankord temporarily deployed to terra-core-deployed-pr-2620 October 16, 2019 15:55 Inactive

ryanthemanuel reviewed Oct 24, 2019

View reviewed changes

ryanthemanuel self-assigned this Oct 24, 2019

updated changelogs and upgrade guide

128dd37

supreethmr requested a review from dkasper-was-taken as a code owner October 25, 2019 08:53

supreethmr and others added 2 commits October 25, 2019 15:02

corrections to changelog and upgradeguide

8b523a7

Merge branch 'master' into Avatar-Provider-Variant

e921aca

ryanthemanuel added the UX Review Ready label Oct 28, 2019

supreethmr and others added 2 commits November 4, 2019 11:13

Merge branch 'master' into Avatar-Provider-Variant

5c4d57f

Update CONTRIBUTORS.md

4da2553

jeremyfuksa approved these changes Nov 4, 2019

View reviewed changes

jeremyfuksa removed UX Review Required UX Review Ready labels Nov 4, 2019

mjhenkes deployed to terra-core-deployed-pr-2620 November 14, 2019 15:58 View deployment

emilyrohrbough added the ⭐ UX Reviewed UX Reviewed and approved. label Jan 13, 2020

StephenEsser changed the title ~~Changed Shared user variant to Generic Avatar variant~~ [terra-avatar] Changed Shared user variant to Generic Avatar variant Jan 24, 2020

rm012685 and others added 3 commits March 27, 2020 10:05

Merge remote-tracking branch 'origin/master' into Avatar-Provider-Var…

0990768

…iant # Conflicts: # packages/terra-avatar/docs/generic.md # packages/terra-avatar/package.json # packages/terra-avatar/src/terra-dev-site/doc/avatar/About.1.doc.mdx

Merge branch 'master' into Avatar-Provider-Variant

c5d1843

Update screenshots

46c15fe

ryanthemanuel merged commit ed26635 into master Mar 28, 2020

ryanthemanuel deleted the Avatar-Provider-Variant branch March 28, 2020 17:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[terra-avatar] Changed Shared user variant to Generic Avatar variant #2620

[terra-avatar] Changed Shared user variant to Generic Avatar variant #2620

supreethmr commented Aug 29, 2019

nramamurth Aug 30, 2019

supreethmr Aug 30, 2019

nramamurth Aug 30, 2019

supreethmr Aug 30, 2019

nramamurth Aug 30, 2019

supreethmr Aug 30, 2019

nramamurth Aug 30, 2019 •

edited

Loading

supreethmr Aug 30, 2019

ShettyAkarsh Aug 30, 2019

ShettyAkarsh Aug 30, 2019

ShettyAkarsh Aug 30, 2019

supreethmr Sep 3, 2019

yuderekyu left a comment

jeremyfuksa commented Oct 10, 2019 •

edited

Loading

ryanthemanuel Oct 24, 2019

supreethmr Oct 25, 2019

jeremyfuksa left a comment

	* `Generic` Variant with props same as `Shared User` and with `Variant` prop to support sub-variants like `single-user`, `shared-user` and `provider`.
	* the `generic` subcomponent that replaces the `sharedUser` subcomponent with a new `variant` prop that can be `single-user`, `shared-user`, or `provider`.


		const cx = classNames.bind(styles);

		const GENERIC_VARIANTS = {

[terra-avatar] Changed Shared user variant to Generic Avatar variant #2620

[terra-avatar] Changed Shared user variant to Generic Avatar variant #2620

Conversation

supreethmr commented Aug 29, 2019

Summary

Additional Details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nramamurth Aug 30, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yuderekyu left a comment

Choose a reason for hiding this comment

jeremyfuksa commented Oct 10, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jeremyfuksa left a comment

Choose a reason for hiding this comment

nramamurth Aug 30, 2019 •

edited

Loading

jeremyfuksa commented Oct 10, 2019 •

edited

Loading