diff --git a/src/components/portrait/portrait.component.tsx b/src/components/portrait/portrait.component.tsx index aa51caef39..87663bd0a2 100644 --- a/src/components/portrait/portrait.component.tsx +++ b/src/components/portrait/portrait.component.tsx @@ -108,18 +108,12 @@ const Portrait = ({ }; const renderComponent = () => { - let portrait = ( - - ); + let portrait = ; if (initials) { portrait = ( - - {initials.toUpperCase()} + + {initials} ); } diff --git a/src/components/portrait/portrait.pw.tsx b/src/components/portrait/portrait.pw.tsx index 9fbf8d787c..575d56a975 100644 --- a/src/components/portrait/portrait.pw.tsx +++ b/src/components/portrait/portrait.pw.tsx @@ -227,8 +227,6 @@ test.describe("Prop checks for Portrait component", () => { "background-color" ); - console.log(backgroundColorTokens); - expect(backgroundColorTokens.toString()).toBe(tokenVal); await expect(portraitPreview(page)).toHaveCSS( "background-color", @@ -255,8 +253,6 @@ test.describe("Prop checks for Portrait component", () => { "color" ); - console.log(colorTokens); - expect(colorTokens.toString()).toBe(tokenVal); await expect(portraitPreview(page)).toHaveCSS("color", color); }); diff --git a/src/components/portrait/portrait.spec.tsx b/src/components/portrait/portrait.spec.tsx index 724952e54a..70a6df7117 100644 --- a/src/components/portrait/portrait.spec.tsx +++ b/src/components/portrait/portrait.spec.tsx @@ -54,7 +54,6 @@ describe("PortraitComponent", () => { overflow: "hidden", border: "1px solid var(--colorsUtilityReadOnly600)", display: "inline-block", - flexShrink: "0", }, wrapper.find(StyledPortraitContainer) ); @@ -411,7 +410,6 @@ describe("PortraitComponent", () => { const expectedProps = { children: "AB", size: "M", - darkBackground: false, }; const testSuccess = (element: JSX.Element) => diff --git a/src/components/portrait/portrait.style.tsx b/src/components/portrait/portrait.style.tsx index d74a6847f2..45f7592632 100644 --- a/src/components/portrait/portrait.style.tsx +++ b/src/components/portrait/portrait.style.tsx @@ -3,7 +3,7 @@ import styled from "styled-components"; import { margin, MarginProps } from "styled-system"; import BaseTheme from "../../style/themes/base"; -import Icon, { IconType } from "../icon"; +import Icon from "../icon"; import { PortraitSizes, PortraitShapes } from "./portrait.component"; import { PORTRAIT_SIZE_PARAMS } from "./portrait.config"; import profileConfigSizes from "../profile/profile.config"; @@ -12,12 +12,11 @@ type StyledPortraitProps = { darkBackground?: boolean; size: PortraitSizes; shape?: PortraitShapes; - type: IconType; onClick?: (ev: React.MouseEvent) => void; }; export const StyledPortraitInitials = styled.div< - Pick + Pick >` font-weight: bold; font-size: ${({ size }) => profileConfigSizes[size].initialSize}; @@ -41,9 +40,7 @@ export const StyledCustomImg = styled.img` // && is used here to increase the specificity // eslint-disable-next-line @typescript-eslint/no-unused-vars -export const StyledIcon = styled(Icon)< - Pick ->` +export const StyledIcon = styled(Icon)>` && { color: inherit; height: inherit; @@ -52,10 +49,11 @@ export const StyledIcon = styled(Icon)< ::before { font-size: ${({ size }) => PORTRAIT_SIZE_PARAMS[size].iconDimensions}px; } + } `; export const StyledPortraitContainer = styled.div< - Pick & MarginProps + StyledPortraitProps & MarginProps >` color: ${({ darkBackground }) => darkBackground @@ -72,7 +70,6 @@ export const StyledPortraitContainer = styled.div< shape === "square" ? "0px" : "var(--borderRadiusCircle)"}; border: 1px solid var(--colorsUtilityReadOnly600); display: inline-block; - flex-shrink: 0; ${({ onClick }) => onClick && "cursor: pointer"} ${margin} `; diff --git a/src/components/profile/component.test-pw.tsx b/src/components/profile/component.test-pw.tsx index 30a431a9c8..6cb2317cae 100644 --- a/src/components/profile/component.test-pw.tsx +++ b/src/components/profile/component.test-pw.tsx @@ -1,11 +1,11 @@ import React from "react"; import Profile, { ProfileProps } from "."; -const EmptyProfileComponent = (props: Partial) => { +export const EmptyProfileComponent = (props: Partial) => { return ; }; -const ProfileComponent = (props: Partial) => { +export const ProfileComponent = (props: Partial) => { return ( ) => { /> ); }; - -export { EmptyProfileComponent, ProfileComponent }; diff --git a/src/components/profile/profile-test.stories.tsx b/src/components/profile/profile-test.stories.tsx index 4b62b5cdf6..4727a82eec 100644 --- a/src/components/profile/profile-test.stories.tsx +++ b/src/components/profile/profile-test.stories.tsx @@ -32,6 +32,7 @@ DefaultStory.story = { initials: "JS", size: PROFILE_SIZES[0], name: "John Smith", + text: "Some other text about John here", src: "", }, }; diff --git a/src/components/profile/profile.component.tsx b/src/components/profile/profile.component.tsx index fbb2bc4377..d5a33f6716 100644 --- a/src/components/profile/profile.component.tsx +++ b/src/components/profile/profile.component.tsx @@ -1,5 +1,4 @@ import React from "react"; -import invariant from "invariant"; import tagComponent from "../../__internal__/utils/helpers/tags/tags"; import { @@ -13,7 +12,7 @@ import { import { filterStyledSystemMarginProps } from "../../style/utils"; import { ProfileSize } from "./profile.config"; -export function acronymize(str?: string) { +function acronymize(str?: string) { if (!str) return ""; const matches = str.match(/\b\w/g); if (!matches) return ""; @@ -55,7 +54,15 @@ export const Profile = ({ }: ProfileProps) => { const getInitials = () => { if (initials) return initials; - return acronymize(name); + return acronymize(name).slice(0, 3).toUpperCase(); + }; + + const commonAvatarProps = { + darkBackground, + alt, + name, + initials: getInitials(), + size, }; const avatar = () => { @@ -63,32 +70,23 @@ export const Profile = ({ return ( ); } - return ( - - ); + return ; }; - invariant( - !(!name && (email || text)), - "The `email` or `text` prop cannot be used without the `name` prop." + - " Please use the `name` prop as well as `email` or `text`." - ); + let useOfNoNameWarnTriggered = false; + + if (!useOfNoNameWarnTriggered && !name && (email || text)) { + useOfNoNameWarnTriggered = true; + console.warn( + "[WARNING] The `email` or `text` prop should not be used without the `name` prop in `Profile`." + + " Please use the `name` prop as well as `email` or `text`." + ); + } const children = () => { if (name) diff --git a/src/components/profile/profile.spec.tsx b/src/components/profile/profile.spec.tsx index b9780bf975..95ecae89f5 100644 --- a/src/components/profile/profile.spec.tsx +++ b/src/components/profile/profile.spec.tsx @@ -2,7 +2,7 @@ import React from "react"; import { shallow, mount, ShallowWrapper, ReactWrapper } from "enzyme"; import MD5 from "crypto-js/md5"; -import Profile, { acronymize } from "./profile.component"; +import Profile from "./profile.component"; import { elementsTagTest, rootTagTest, @@ -17,6 +17,7 @@ import { } from "./profile.style"; import { StyledCustomImg, + StyledPortraitContainer, StyledPortraitInitials, } from "../portrait/portrait.style"; import { StyledLink } from "../link/link.style"; @@ -29,6 +30,108 @@ import profileConfigSizes, { ProfileSize } from "./profile.config"; describe("Profile", () => { let instance: ShallowWrapper | ReactWrapper; + let loggerSpy: jest.SpyInstance | jest.Mock; + + beforeEach(() => { + loggerSpy = jest.spyOn(console, "warn"); + instance = mount(); + }); + + afterEach(() => { + loggerSpy.mockRestore(); + instance.unmount(); + }); + + afterAll(() => { + loggerSpy.mockClear(); + jest.clearAllMocks(); + }); + + describe("console warnings", () => { + const warningMessage = + "[WARNING] The `email` or `text` prop should not be used without the `name` prop in `Profile`." + + " Please use the `name` prop as well as `email` or `text`."; + + it("validates a warning is logged in the console if `name` is not passed but `email` is", () => { + instance.setProps({ email: "johm@doe.com" }); + + expect(loggerSpy).toHaveBeenCalledWith(warningMessage); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is logged in the console if `name` is not passed but `text` is", () => { + instance.setProps({ text: "Some other text about John here" }); + + expect(loggerSpy).toHaveBeenCalledWith(warningMessage); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is logged in the console if `name` is not passed but `text` and `email` are", () => { + instance.setProps({ + text: "Some other text about John here", + email: "john@doe.com", + }); + + expect(loggerSpy).toHaveBeenCalledWith(warningMessage); + + expect(loggerSpy).toHaveBeenCalledTimes(1); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is not logged in the console if `name` is passed and `email` is not", () => { + instance.setProps({ + name: "John Doe", + text: "Some other text about John here", + }); + + expect(loggerSpy).toHaveBeenCalledTimes(0); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is not logged in the console if `name` is passed and `text` is not", () => { + instance.setProps({ name: "John Doe", email: "john@doe.com" }); + + expect(loggerSpy).toHaveBeenCalledTimes(0); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is not logged in the console if `name` is passed and both `text` and `email` are not", () => { + instance.setProps({ name: "John Doe" }); + + expect(loggerSpy).toHaveBeenCalledTimes(0); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is not logged in the console if `name` is passed and both `text` and `email` are", () => { + instance.setProps({ + name: "John Doe", + text: "Some other text about John here", + email: "john@doe.com", + }); + + expect(loggerSpy).toHaveBeenCalledTimes(0); + + loggerSpy.mockRestore(); + }); + + it("validates a warning is not logged in the console if `name`,`text` and `email` are all not passed", () => { + instance.setProps({}); + + expect(loggerSpy).toHaveBeenCalledTimes(0); + + loggerSpy.mockRestore(); + }); + }); describe("render", () => { beforeEach(() => { @@ -43,34 +146,48 @@ describe("Profile", () => { }); describe("initials", () => { - it("calculates the initials when not provided", () => { - instance = shallow(); + it("calculates the correct initial when only a name is provided", () => { + instance = shallow(); expect(instance.find(ProfileAvatarStyle).props().initials).toEqual( "JD" ); }); - it("returns an empty string if no name is found", () => { - instance = shallow(); - expect(instance.find(ProfileAvatarStyle).props().initials).toEqual(""); - }); + it.each([ + ["John Doe", "JD"], + ["John Dan Doe", "JDD"], + ])( + "calculates the correct initials when only a name is provided", + (name, initials) => { + instance = shallow(); + expect(instance.find(ProfileAvatarStyle).props().initials).toEqual( + initials + ); + } + ); - test("acronymize function generates correct acronym", () => { - const input = "World Wide Web"; - const result = acronymize(input); - expect(result).toEqual("WWW"); - }); + it.each([ + "John David Daniel Doe", + "John David Daniel Damien Doe", + "John David Daniel Damien Dack Doe", + ])( + "calculates the correct maximum initials of three, when only a long name is provided", + (name) => { + instance = shallow(); + expect(instance.find(ProfileAvatarStyle).props().initials).toEqual( + "JDD" + ); + } + ); - test("acronymize function returns empty string if passed string is empty", () => { - const input = ""; - const result = acronymize(input); - expect(result).toEqual(""); + it("returns an empty initials string if no name is provided", () => { + instance = shallow(); + expect(instance.find(ProfileAvatarStyle).props().initials).toEqual(""); }); - test("acronymize function returns empty string if passed string as no word boundaries are identified", () => { - const input = "!@£$%"; - const result = acronymize(input); - expect(result).toEqual(""); + it("returns an empty initials string if provided name has no identifiable word boundaries", () => { + instance = shallow(); + expect(instance.find(ProfileAvatarStyle).props().initials).toEqual(""); }); }); @@ -113,107 +230,6 @@ describe("Profile", () => { instance = mount(); expect(instance.find(StyledPortraitInitials).text()).toBe("JD"); }); - - it("validates an invariant is thrown if `name` is not passed but `email` is", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => instance.setProps({ email: "john@doe.com" })).toThrow( - "The `email` or `text` prop cannot be used without the `name` prop." + - " Please use the `name` prop as well as `email` or `text`." - ); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is thrown if `name` is not passed but `text` is", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => - instance.setProps({ text: "Some other text about John here" }) - ).toThrow( - "The `email` or `text` prop cannot be used without the `name` prop." + - " Please use the `name` prop as well as `email` or `text`." - ); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is thrown if `name` is not passed but `text` and `email` are", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => - instance.setProps({ - text: "Some other text about John here", - email: "john@doe.com", - }) - ).toThrow( - "The `email` or `text` prop cannot be used without the `name` prop." + - " Please use the `name` prop as well as `email` or `text`." - ); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is not thrown if `name` is passed and `email` is not", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => - instance.setProps({ - name: "John Doe", - text: "Some other text about John here", - }) - ).not.toThrow(); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is not thrown if `name` is passed and `text` is not", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => - instance.setProps({ name: "John Doe", email: "john@doe.com" }) - ).not.toThrow(); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is not thrown if `name` is passed and both `text` and `email` are not", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => instance.setProps({ name: "John Doe" })).not.toThrow(); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is not thrown if `name` is passed and both `text` and `email` are", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => - instance.setProps({ - name: "John Doe", - email: "john@doe.com", - text: "Some other text about John here", - }) - ).not.toThrow(); - - consoleSpy.mockRestore(); - }); - - it("validates an invariant is not thrown if `name`,`text` and `email` are all not passed", () => { - const consoleSpy = jest.spyOn(console, "error"); - consoleSpy.mockImplementation(() => {}); - - expect(() => instance.setProps({})).not.toThrow(); - - consoleSpy.mockRestore(); - }); }); }); @@ -233,12 +249,21 @@ describe("Profile", () => { whiteSpace: "nowrap", display: "flex", flexDirection: "row", - flexShrink: "0", }, wrapper.find(ProfileStyle) ); }); + it("applies expected styles to Portrait container", () => { + assertStyleMatch( + { + flexShrink: "0", + }, + wrapper.find(ProfileStyle), + { modifier: `${StyledPortraitContainer}` } + ); + }); + it("applies expected styles to Profile name", () => { assertStyleMatch( { @@ -396,86 +421,86 @@ describe("Profile", () => { elementsTagTest(wrapper, ["email", "name"]); }); }); -}); -describe("src", () => { - it('should render a user image when a "src" prop is provided', () => { - const wrapper = shallow( - - ); - expect(wrapper.find(ProfileAvatarStyle).props()["data-element"]).toEqual( - "user-image" - ); - }); + describe("src", () => { + it('should render a user image when a "src" prop is provided', () => { + const wrapper = shallow( + + ); + expect(wrapper.find(ProfileAvatarStyle).props()["data-element"]).toEqual( + "user-image" + ); + }); - it("alt text on user image should be provided via alt prop", () => { - const wrapper = mount( - - ); - expect(wrapper.find(StyledCustomImg).prop("alt")).toBe("John V Doe"); - }); + it("alt text on user image should be provided via alt prop", () => { + const wrapper = mount( + + ); + expect(wrapper.find(StyledCustomImg).prop("alt")).toBe("John V Doe"); + }); - it("alt text on user image should be the name passed, if no alt prop is provided", () => { - const wrapper = mount( - - ); - expect(wrapper.find(StyledCustomImg).prop("alt")).toEqual("John Doe"); + it("alt text on user image should be the name passed, if no alt prop is provided", () => { + const wrapper = mount( + + ); + expect(wrapper.find(StyledCustomImg).prop("alt")).toEqual("John Doe"); + }); }); -}); -describe("gravatar", () => { - const gravatarEmail = "chris.barber@sage.com"; - const base = "https://www.gravatar.com/avatar/"; - const hash = MD5(gravatarEmail); - const dimensions = 40; - const expectedSrc = `${base}${hash}?s=${dimensions}&d=404`; + describe("gravatar", () => { + const gravatarEmail = "chris.barber@sage.com"; + const base = "https://www.gravatar.com/avatar/"; + const hash = MD5(gravatarEmail); + const dimensions = 40; + const expectedSrc = `${base}${hash}?s=${dimensions}&d=404`; - it("should render a gravatar if passed email matches gravatar account", () => { - const wrapper = mount( - - ); + it("should render a gravatar if passed email matches gravatar account", () => { + const wrapper = mount( + + ); - expect(wrapper.find("img").prop("src")).toEqual(expectedSrc); - }); + expect(wrapper.find("img").prop("src")).toEqual(expectedSrc); + }); - it("alt text on gravatar should be provided via alt prop", () => { - const wrapper = mount( - - ); + it("alt text on gravatar should be provided via alt prop", () => { + const wrapper = mount( + + ); - expect(wrapper.find("img").prop("alt")).toEqual("Mr Chris Barber"); - }); + expect(wrapper.find("img").prop("alt")).toEqual("Mr Chris Barber"); + }); - it("alt text on user image should be the name passed if no alt prop is provided", () => { - const wrapper = mount( - - ); + it("alt text on user image should be the name passed if no alt prop is provided", () => { + const wrapper = mount( + + ); - expect(wrapper.find("img").prop("alt")).toEqual("Chris Barber"); + expect(wrapper.find("img").prop("alt")).toEqual("Chris Barber"); + }); }); -}); -describe("styled-system", () => { - testStyledSystemMargin((props) => ( - - )); + describe("styled-system", () => { + testStyledSystemMargin((props) => ( + + )); + }); }); diff --git a/src/components/profile/profile.stories.mdx b/src/components/profile/profile.stories.mdx index dfc864c64a..129a4aeced 100644 --- a/src/components/profile/profile.stories.mdx +++ b/src/components/profile/profile.stories.mdx @@ -63,14 +63,6 @@ To use an image, simply pass any valid image URL as a `src` prop. -### Gravatar - -Profile also supports `gravatar`, to use it simply pass your email address registered with Gravatar as the `email` prop. - - - - - ### Sizes diff --git a/src/components/profile/profile.stories.tsx b/src/components/profile/profile.stories.tsx index 6f47f319e8..71d029fe3c 100644 --- a/src/components/profile/profile.stories.tsx +++ b/src/components/profile/profile.stories.tsx @@ -43,14 +43,6 @@ export const Src: ComponentStory = () => ( /> ); -export const Gravatar: ComponentStory = () => ( - -); - export const Sizes: ComponentStory = () => { return ( <> diff --git a/src/components/profile/profile.style.ts b/src/components/profile/profile.style.ts index d57fbdf93a..28da7868a9 100644 --- a/src/components/profile/profile.style.ts +++ b/src/components/profile/profile.style.ts @@ -5,6 +5,7 @@ import Portrait from "../portrait"; import baseTheme from "../../style/themes/base"; import profileConfigSizes, { ProfileSize } from "./profile.config"; import Link from "../link"; +import { StyledPortraitContainer } from "../portrait/portrait.style"; interface ProfileSProps { size?: ProfileSize; @@ -40,8 +41,11 @@ const ProfileStyle = styled.div< darkBackground ? "var(--colorsUtilityYin090)" : "transparent"}; display: flex; flex-direction: row; - flex-shrink: 0; ${margin} + + ${StyledPortraitContainer} { + flex-shrink: 0; + } `; const ProfileDetailsStyle = styled.div>`