Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add placeholder for custom svg icon for SSR [prerelease] #2485

Merged
merged 6 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
Ocean
</div>
Expand All @@ -118,7 +117,6 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
Blue
</div>
Expand All @@ -138,7 +136,6 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
Purple
</div>
Expand All @@ -158,7 +155,6 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
Red
</div>
Expand All @@ -178,7 +174,6 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
Orange
</div>
Expand All @@ -198,7 +193,6 @@ exports[`Dropdown renders correctly snapshot driver should open menu on click if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
Yellow
</div>
Expand Down Expand Up @@ -986,7 +980,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
1
</div>
Expand All @@ -1012,7 +1005,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
2
</div>
Expand All @@ -1038,7 +1030,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
3
</div>
Expand All @@ -1064,7 +1055,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
4
</div>
Expand All @@ -1090,7 +1080,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
5
</div>
Expand All @@ -1116,7 +1105,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
6
</div>
Expand All @@ -1142,7 +1130,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
7
</div>
Expand All @@ -1168,7 +1155,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
8
</div>
Expand All @@ -1194,7 +1180,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
9
</div>
Expand All @@ -1220,7 +1205,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
10
</div>
Expand All @@ -1246,7 +1230,6 @@ exports[`Dropdown renders correctly snapshot driver should use virtualization if
<div
class="typography inherit start singleLineEllipsis text text2Normal"
data-testid="text"
style="--text-clamp-lines: 1;"
>
11
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import useIconScreenReaderAccessProps from "../../../hooks/useIconScreenReaderAc
import VibeComponentProps from "../../../types/VibeComponentProps";
import { ComponentDefaultTestId } from "../../../tests/constants";
import { getTestId } from "../../../tests/test-ids-utils";
import { useIsMounted } from "../../../hooks/ssr/useIsMounted";

function modifySvgCode(svg: string, color = "currentColor") {
return svg.replace(/fill=".*?"/g, `fill="${color}"`);
Expand Down Expand Up @@ -41,6 +42,8 @@ const CustomSvgIcon: FunctionComponent<CustomSvgIconProps> = ({
isDecorationOnly: ariaHidden
});

const isMounted = useIsMounted();

const svgProcessor = useCallback(
(svg: string) => {
if (replaceToCurrentColor) return modifySvgCode(svg, "currentColor");
Expand All @@ -55,6 +58,10 @@ const CustomSvgIcon: FunctionComponent<CustomSvgIconProps> = ({
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const SVGComponent = SVG as React.FC<any>;

if (!isMounted) {
// placeholder for server side rendering
return <div className={className} id={id}></div>;
}
return (
<SVGComponent
innerRef={ref}
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/components/Icon/Icon.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export interface IconProps extends VibeComponentProps {
*/
clickable?: boolean;
/**
mo * Icon aria label [aria label](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-label)
* Icon aria label [aria label](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-label)
*/
iconLabel?: string;
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ Snapshot Diff:
+ Current Render: {"content":{"key":null,"ref":null,"props":{"children":[{"key":null,"ref":null,"props":{"title":"ModalHeader Heading"},"_owner":null,"_store":{}},{"type":"p","key":null,"ref":null,"props":{"children":"content"},"_owner":null,"_store":{}}]},"_owner":null,"_store":{}}}

@@ --- --- @@
class="typography primary start text text1Normal container"
data-testid="modal-content"
style="--text-clamp-lines: 1;"
>
+ <div
+ class="container"
Expand Down Expand Up @@ -97,8 +97,8 @@ Snapshot Diff:
+ Current Render: {"content":{"key":null,"ref":null,"props":{"children":[{"key":null,"ref":null,"props":{"title":"ModalHeader Heading"},"_owner":null,"_store":{}},{"key":null,"ref":null,"props":{"children":{"type":"p","key":null,"ref":null,"props":{"children":"content"},"_owner":null,"_store":{}}},"_owner":null,"_store":{}},{"key":null,"ref":null,"props":{"children":{"type":"p","key":null,"ref":null,"props":{"children":"footer"},"_owner":null,"_store":{}}},"_owner":null,"_store":{}}]},"_owner":null,"_store":{}}}

@@ --- --- @@
class="typography primary start text text1Normal container"
data-testid="modal-content"
style="--text-clamp-lines: 1;"
>
+ <div
+ class="container"
Expand Down Expand Up @@ -154,7 +154,6 @@ Snapshot Diff:
+ <div
+ class="typography primary start text text1Normal container"
+ data-testid="modal-content"
+ style="--text-clamp-lines: 1;"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seem like an unwanted change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is ok - I've removed text-clamp if single ellipsis line

+ >
<p>
content
Expand Down Expand Up @@ -246,7 +245,6 @@ exports[`Modal with base props 1`] = `
<div
class="typography primary start text text1Normal container"
data-testid="modal-content"
style="--text-clamp-lines: 1;"
>
<p>
content
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,11 @@ exports[`Snapshot tests TipseenTitle should render correctly with given text 1`]
className="typography inherit start multiLineEllipsis text text1Bold title"
data-testid="tipseen-title"
role="heading"
style={
{
"--text-clamp-lines": "2",
}
}
>
I'm a title
</div>
Expand Down
21 changes: 13 additions & 8 deletions packages/core/src/components/Typography/TypographyHooks.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,20 @@ import { TooltipProps } from "../Tooltip/Tooltip";
import styles from "./Typography.module.scss";

export function useEllipsisClass(ellipsis: boolean, maxLines = 1) {
let ellipsisClass: string;
// If component contains ellipsis return the fit ellipsis class
if (ellipsis) {
ellipsisClass = maxLines > 1 ? styles.multiLineEllipsis : styles.singleLineEllipsis;
}

const result = useMemo(() => {
return { class: ellipsisClass, style: { "--text-clamp-lines": maxLines.toString() } };
}, [ellipsisClass, maxLines]);
let ellipsisClass: string;
let style: Record<string, string>;
// If component contains ellipsis return the fit ellipsis class
if (ellipsis) {
ellipsisClass = maxLines > 1 ? styles.multiLineEllipsis : styles.singleLineEllipsis;
if (maxLines > 1) {
// not relevant for single line ellipsis
style = { "--text-clamp-lines": maxLines.toString() };
}
}

return { class: ellipsisClass, style };
}, [ellipsis, maxLines]);
return result;
}

Expand Down
13 changes: 13 additions & 0 deletions packages/core/src/hooks/ssr/useIsMounted.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { useEffect, useState } from "react";

// The hook will return true only on client side and not on server side
export const useIsMounted = () => {
const [isMounted, setIsMounted] = useState(false);
useEffect(() => {
setIsMounted(true);
return () => {
setIsMounted(false);
};
}, []);
return isMounted;
};
1 change: 1 addition & 0 deletions packages/core/src/utils/ssr-utils.ts
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
export const isClient = () => typeof window !== "undefined";
export const isServer = () => !isClient();