Skip to content

Commit

Permalink
chore: address comments from review
Browse files Browse the repository at this point in the history
  • Loading branch information
tomdavies73 committed Nov 3, 2023
1 parent d0e219b commit cb4c870
Show file tree
Hide file tree
Showing 13 changed files with 210 additions and 263 deletions.
12 changes: 3 additions & 9 deletions src/components/portrait/portrait.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -108,18 +108,12 @@ const Portrait = ({
};

const renderComponent = () => {
let portrait = (
<StyledIcon type={iconType} size={size} darkBackground={darkBackground} />
);
let portrait = <StyledIcon type={iconType} size={size} />;

if (initials) {
portrait = (
<StyledPortraitInitials
size={size}
darkBackground={darkBackground}
data-element="initials"
>
{initials.toUpperCase()}
<StyledPortraitInitials size={size} data-element="initials">
{initials.slice(0, 3).toUpperCase()}
</StyledPortraitInitials>
);
}
Expand Down
20 changes: 16 additions & 4 deletions src/components/portrait/portrait.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@ test.describe("Prop checks for Portrait component", () => {
});
});

[
["SPMMMMM", "SPM"],
["JMMMMM", "JMM"],
["ARRRR", "ARR"],
["MJJJ", "MJJ"],
].forEach(([passInitials, maxInitials]) => {
test(`should render with a maximum of three initials when passed initials are ${passInitials} `, async ({
mount,
page,
}) => {
await mount(<PortraitDefaultComponent initials={passInitials} />);

await expect(portraitInitials(page)).toHaveText(maxInitials);
});
});

test("should render with gravatar prop", async ({ mount, page }) => {
await mount(<PortraitDefaultComponent gravatar="chris.barber@sage.com" />);

Expand Down Expand Up @@ -227,8 +243,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",
Expand All @@ -255,8 +269,6 @@ test.describe("Prop checks for Portrait component", () => {
"color"
);

console.log(colorTokens);

expect(colorTokens.toString()).toBe(tokenVal);
await expect(portraitPreview(page)).toHaveCSS("color", color);
});
Expand Down
2 changes: 0 additions & 2 deletions src/components/portrait/portrait.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ describe("PortraitComponent", () => {
overflow: "hidden",
border: "1px solid var(--colorsUtilityReadOnly600)",
display: "inline-block",
flexShrink: "0",
},
wrapper.find(StyledPortraitContainer)
);
Expand Down Expand Up @@ -411,7 +410,6 @@ describe("PortraitComponent", () => {
const expectedProps = {
children: "AB",
size: "M",
darkBackground: false,
};

const testSuccess = (element: JSX.Element) =>
Expand Down
8 changes: 4 additions & 4 deletions src/components/portrait/portrait.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ export const WithTooltip: ComponentStory<typeof Portrait> = () => (

export const Sizes: ComponentStory<typeof Portrait> = () => {
return (
<>
<Box display="flex" alignItems="baseline">
{(["XS", "S", "M", "ML", "L", "XL", "XXL"] as const).map((size) => (
<Portrait key={size} size={size} />
))}
</>
</Box>
);
};

Expand All @@ -61,10 +61,10 @@ export const DarkBackground: ComponentStory<typeof Portrait> = () => (
);

export const WithMargin: ComponentStory<typeof Portrait> = () => (
<>
<Box display="flex" alignItems="baseline">
<Portrait m={3} />
<Portrait darkBackground m={2} />
<Portrait shape="circle" m="25px" />
<Portrait size="L" m="30px" />
</>
</Box>
);
13 changes: 5 additions & 8 deletions src/components/portrait/portrait.style.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -12,12 +12,11 @@ type StyledPortraitProps = {
darkBackground?: boolean;
size: PortraitSizes;
shape?: PortraitShapes;
type: IconType;
onClick?: (ev: React.MouseEvent<HTMLElement>) => void;
};

export const StyledPortraitInitials = styled.div<
Pick<StyledPortraitProps, "darkBackground" | "size">
Pick<StyledPortraitProps, "size">
>`
font-weight: bold;
font-size: ${({ size }) => profileConfigSizes[size].initialSize};
Expand All @@ -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<StyledPortraitProps, "darkBackground" | "size">
>`
export const StyledIcon = styled(Icon)<Pick<StyledPortraitProps, "size">>`
&& {
color: inherit;
height: inherit;
Expand All @@ -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<StyledPortraitProps, "darkBackground" | "size" | "shape"> & MarginProps
StyledPortraitProps & MarginProps
>`
color: ${({ darkBackground }) =>
darkBackground
Expand All @@ -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}
`;
Expand Down
6 changes: 2 additions & 4 deletions src/components/profile/component.test-pw.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from "react";
import Profile, { ProfileProps } from ".";

const EmptyProfileComponent = (props: Partial<ProfileProps>) => {
export const EmptyProfileComponent = (props: Partial<ProfileProps>) => {
return <Profile {...props} />;
};

const ProfileComponent = (props: Partial<ProfileProps>) => {
export const ProfileComponent = (props: Partial<ProfileProps>) => {
return (
<Profile
email="email@email.com"
Expand All @@ -16,5 +16,3 @@ const ProfileComponent = (props: Partial<ProfileProps>) => {
/>
);
};

export { EmptyProfileComponent, ProfileComponent };
1 change: 1 addition & 0 deletions src/components/profile/profile-test.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ DefaultStory.story = {
initials: "JS",
size: PROFILE_SIZES[0],
name: "John Smith",
text: "Some other text about John here",
src: "",
},
};
Expand Down
44 changes: 21 additions & 23 deletions src/components/profile/profile.component.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import React from "react";
import invariant from "invariant";

import tagComponent from "../../__internal__/utils/helpers/tags/tags";
import {
Expand All @@ -13,13 +12,15 @@ 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 "";
return matches.join("");
}

let useOfNoNameWarnTriggered = false;

export interface ProfileProps {
/** [Legacy] A custom class name for the component */
className?: string;
Expand Down Expand Up @@ -55,40 +56,37 @@ 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 = () => {
if (src) {
return (
<ProfileAvatarStyle
src={src}
darkBackground={darkBackground}
alt={alt}
name={name}
initials={getInitials()}
size={size}
data-element="user-image"
{...commonAvatarProps}
/>
);
}
return (
<ProfileAvatarStyle
initials={getInitials()}
darkBackground={darkBackground}
gravatar={email}
alt={alt}
name={name}
size={size}
/>
);
return <ProfileAvatarStyle gravatar={email} {...commonAvatarProps} />;
};

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`."
);
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)
Expand Down
3 changes: 2 additions & 1 deletion src/components/profile/profile.pw.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,12 @@ test.describe("Prop checks for Profile component", () => {
});

[
["World Wide Web Web", "WWW"],
["World Wide Web", "WWW"],
["John Doe", "JD"],
["Foo", "F"],
].forEach(([name, initials]) => {
test(`should render with correct initials when nname prop is passed as ${name} and no other icon, src or gravatar is passed`, async ({
test(`should render with correct initials when name prop is passed as ${name} and no other icon, src or gravatar is passed`, async ({
mount,
page,
}) => {
Expand Down
Loading

0 comments on commit cb4c870

Please sign in to comment.