From baf2dbde9c881e0693003a8caf44c3a87e9830c1 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Fri, 15 Sep 2023 16:10:55 +0200 Subject: [PATCH 01/11] feat(Tooltip): support ReactElement as content --- packages/components/tooltip/README.mdx | 2 ++ .../components/tooltip/src/Tooltip.test.tsx | 23 ++++++++++++++ packages/components/tooltip/src/Tooltip.tsx | 31 +++++++++++++++---- .../tooltip/stories/Tooltip.stories.tsx | 26 ++++++++++++++++ .../forma-36-codemod/transforms/v4-tooltip.js | 12 ------- 5 files changed, 76 insertions(+), 18 deletions(-) diff --git a/packages/components/tooltip/README.mdx b/packages/components/tooltip/README.mdx index 993634be9a..e53d41f635 100644 --- a/packages/components/tooltip/README.mdx +++ b/packages/components/tooltip/README.mdx @@ -50,3 +50,5 @@ import { Tooltip } from '@contentful/f36-tooltip'; Hover me ``` + +- When using `ReactElement` as the content, it's recommended to use the `ScreenReaderOnly` component when displaying critical information. diff --git a/packages/components/tooltip/src/Tooltip.test.tsx b/packages/components/tooltip/src/Tooltip.test.tsx index 461770ada0..7b3b1b89ff 100644 --- a/packages/components/tooltip/src/Tooltip.test.tsx +++ b/packages/components/tooltip/src/Tooltip.test.tsx @@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event'; import { axe } from '@/scripts/test/axeHelper'; import { Tooltip } from './Tooltip'; +import { Paragraph } from '@contentful/f36-typography'; jest.mock('@contentful/f36-core', () => { const actual = jest.requireActual('@contentful/f36-core'); @@ -126,4 +127,26 @@ describe('Tooltip', () => { expect(results).toHaveNoViolations(); }); + + it('render a React Element as children', async () => { + const user = userEvent.setup(); + + const { container } = render( + Ich bin ein Paragraph} + > + Hover me + , + ); + await user.hover(screen.getByText('Hover me')); + + const results = await axe(container); + + expect(results).toHaveNoViolations(); + expect(screen.getByRole('tooltip').textContent).toBe( + 'Ich bin ein Paragraph', + ); + }); }); diff --git a/packages/components/tooltip/src/Tooltip.tsx b/packages/components/tooltip/src/Tooltip.tsx index 2cfd3704cb..090c890ec4 100644 --- a/packages/components/tooltip/src/Tooltip.tsx +++ b/packages/components/tooltip/src/Tooltip.tsx @@ -18,6 +18,28 @@ import { getStyles } from './Tooltip.styles'; export type TooltipPlacement = Placement; +type TextContentTooltip = { + /** + * Content of the tooltip + */ + content?: string; + /** + * Accesible label property, only required when using ReactElement as content + */ + label?: string; +}; + +type RichContentTooltip = { + /** + * Content of the tooltip + */ + content?: React.ReactElement; + /** + * Accesible label property, only required when using ReactElement as content + */ + label: string; +}; + export interface TooltipProps extends CommonProps { /** * Child nodes to be rendered in the component and that will show the tooltip when they are hovered @@ -27,10 +49,6 @@ export interface TooltipProps extends CommonProps { * HTML element used to wrap the target of the tooltip */ as?: React.ElementType; - /** - * Content of the tooltip - */ - content?: string; /** * A unique id of the tooltip */ @@ -101,6 +119,7 @@ export const Tooltip = ({ className, as: HtmlTag = 'span', content, + label, id, isVisible = false, hideDelay = 0, @@ -117,7 +136,7 @@ export const Tooltip = ({ usePortal = false, isDisabled = false, ...otherProps -}: TooltipProps) => { +}: TooltipProps & (TextContentTooltip | RichContentTooltip)) => { const styles = getStyles(); const [show, setShow] = useState(isVisible); const tooltipId = useId(id, 'tooltip'); @@ -210,7 +229,7 @@ export const Tooltip = ({ }} {...attributes.popper} > - {content} + {content} { ); }; + +export const WithReactElement = () => { + return ( + <> + + With React Elements as children + + + + +

I'm a React Element

+
+ } + > + Hover me + + + + ); +}; diff --git a/packages/forma-36-codemod/transforms/v4-tooltip.js b/packages/forma-36-codemod/transforms/v4-tooltip.js index 92a3a7feb3..1d12240595 100644 --- a/packages/forma-36-codemod/transforms/v4-tooltip.js +++ b/packages/forma-36-codemod/transforms/v4-tooltip.js @@ -1,19 +1,7 @@ const { modifyPropsCodemod } = require('./common/modify-props-codemod'); -const { hasProperty, getProperty } = require('../utils'); module.exports = modifyPropsCodemod({ componentName: 'Tooltip', - beforeRename: (attributes) => { - if ( - hasProperty(attributes, { propertyName: 'content' }) && - typeof getProperty(attributes, { propertyName: 'content' } !== 'string') - ) { - console.error( - 'Value of property "content" on Tooltip component should be a string.', - ); - } - return attributes; - }, renameMap: { place: 'placement', containerElement: 'as', From 58e769cb407751f7458be7031c93778a2d634570 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Fri, 15 Sep 2023 16:27:50 +0200 Subject: [PATCH 02/11] fix: linter --- packages/components/tooltip/stories/Tooltip.stories.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/tooltip/stories/Tooltip.stories.tsx b/packages/components/tooltip/stories/Tooltip.stories.tsx index ba1eb70612..7f9091b6c8 100644 --- a/packages/components/tooltip/stories/Tooltip.stories.tsx +++ b/packages/components/tooltip/stories/Tooltip.stories.tsx @@ -228,7 +228,7 @@ export const WithReactElement = () => { label="Render a paragrah with a React Element" content={ -

I'm a React Element

+

I am a React Element

} > From e45c9afd54d56c2a11df8e3436e11c0d1210ea24 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Fri, 15 Sep 2023 16:44:15 +0200 Subject: [PATCH 03/11] fix: export new properties --- packages/components/tooltip/src/Tooltip.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/components/tooltip/src/Tooltip.tsx b/packages/components/tooltip/src/Tooltip.tsx index 090c890ec4..a70b30000c 100644 --- a/packages/components/tooltip/src/Tooltip.tsx +++ b/packages/components/tooltip/src/Tooltip.tsx @@ -40,7 +40,7 @@ type RichContentTooltip = { label: string; }; -export interface TooltipProps extends CommonProps { +export type TooltipProps = CommonProps & { /** * Child nodes to be rendered in the component and that will show the tooltip when they are hovered */ @@ -112,7 +112,7 @@ export interface TooltipProps extends CommonProps { * @default false */ isDisabled?: boolean; -} +} & (TextContentTooltip | RichContentTooltip); export const Tooltip = ({ children, @@ -136,7 +136,7 @@ export const Tooltip = ({ usePortal = false, isDisabled = false, ...otherProps -}: TooltipProps & (TextContentTooltip | RichContentTooltip)) => { +}: TooltipProps) => { const styles = getStyles(); const [show, setShow] = useState(isVisible); const tooltipId = useId(id, 'tooltip'); From cf80edb58371c38e2885b2b31f370f7642db67ce Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Fri, 15 Sep 2023 17:07:05 +0200 Subject: [PATCH 04/11] fix: make types match --- packages/components/avatar/src/Avatar/Avatar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/avatar/src/Avatar/Avatar.tsx b/packages/components/avatar/src/Avatar/Avatar.tsx index 77c1f56fcb..789bc8b084 100644 --- a/packages/components/avatar/src/Avatar/Avatar.tsx +++ b/packages/components/avatar/src/Avatar/Avatar.tsx @@ -20,7 +20,7 @@ export interface AvatarProps extends CommonProps { /** * A tooltipProps attribute used to conditionally render the tooltip around root element */ - tooltipProps?: Omit; + tooltipProps?: TooltipProps; variant?: Variant; colorVariant?: ColorVariant; icon?: React.ReactElement; From 856b396e7970bc19d808267393109fbdbc595054 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Mon, 18 Sep 2023 14:43:40 +0200 Subject: [PATCH 05/11] fix: revert change --- packages/components/avatar/src/Avatar/Avatar.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/components/avatar/src/Avatar/Avatar.tsx b/packages/components/avatar/src/Avatar/Avatar.tsx index 789bc8b084..77c1f56fcb 100644 --- a/packages/components/avatar/src/Avatar/Avatar.tsx +++ b/packages/components/avatar/src/Avatar/Avatar.tsx @@ -20,7 +20,7 @@ export interface AvatarProps extends CommonProps { /** * A tooltipProps attribute used to conditionally render the tooltip around root element */ - tooltipProps?: TooltipProps; + tooltipProps?: Omit; variant?: Variant; colorVariant?: ColorVariant; icon?: React.ReactElement; From 0db2698fc386f5ee2f9ff2f24fae0203418f7fc9 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Mon, 18 Sep 2023 14:58:10 +0200 Subject: [PATCH 06/11] fix: try to fix the type issue --- packages/components/tooltip/src/Tooltip.tsx | 45 +++++++++++---------- 1 file changed, 23 insertions(+), 22 deletions(-) diff --git a/packages/components/tooltip/src/Tooltip.tsx b/packages/components/tooltip/src/Tooltip.tsx index a70b30000c..6f64b6d322 100644 --- a/packages/components/tooltip/src/Tooltip.tsx +++ b/packages/components/tooltip/src/Tooltip.tsx @@ -5,6 +5,7 @@ import React, { type MouseEvent, type FocusEvent, type CSSProperties, + ReactElement, } from 'react'; import { usePopper } from 'react-popper'; import type { Placement } from '@popperjs/core'; @@ -18,27 +19,27 @@ import { getStyles } from './Tooltip.styles'; export type TooltipPlacement = Placement; -type TextContentTooltip = { - /** - * Content of the tooltip - */ - content?: string; - /** - * Accesible label property, only required when using ReactElement as content - */ - label?: string; -}; - -type RichContentTooltip = { - /** - * Content of the tooltip - */ - content?: React.ReactElement; - /** - * Accesible label property, only required when using ReactElement as content - */ - label: string; -}; +type WithEnhancedContent = + | { + /** + * Content of the tooltip + */ + content: ReactElement; + /** + * Accesible label property, only required when using ReactElement as content + */ + label: string; + } + | { + /** + * Content of the tooltip + */ + content: string; + /** + * Accesible label property, only required when using ReactElement as content + */ + label?: string; + }; export type TooltipProps = CommonProps & { /** @@ -112,7 +113,7 @@ export type TooltipProps = CommonProps & { * @default false */ isDisabled?: boolean; -} & (TextContentTooltip | RichContentTooltip); +} & WithEnhancedContent; export const Tooltip = ({ children, From 9154dee28f7d4268695d57c5301c8cc50eee6348 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Mon, 18 Sep 2023 15:21:40 +0200 Subject: [PATCH 07/11] fix: pr comments --- packages/components/tooltip/README.mdx | 4 ++-- .../tooltip/stories/Tooltip.stories.tsx | 16 ++++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/components/tooltip/README.mdx b/packages/components/tooltip/README.mdx index e53d41f635..63f9cdb34f 100644 --- a/packages/components/tooltip/README.mdx +++ b/packages/components/tooltip/README.mdx @@ -39,6 +39,8 @@ import { Tooltip } from '@contentful/f36-tooltip'; ## Content guidelines - Use short and clear messages as the `Tooltip`’s content +- The Tooltip component allows you to pass React elements as content. However, this should be used with care. ReactElement as content is best suited for text formatting purposes. It should not be used for interactive elements like links, buttons, or form elements. Additionally, extensive images should be avoided, except for text-decorator icons that give semantic meaning to the text shown in the tooltip (e.g., warning signs or other relevant icons). + - When using `ReactElement` as the content, it's recommended to use the `ScreenReaderOnly` component when displaying critical information. ## Accessibility @@ -50,5 +52,3 @@ import { Tooltip } from '@contentful/f36-tooltip'; Hover me ``` - -- When using `ReactElement` as the content, it's recommended to use the `ScreenReaderOnly` component when displaying critical information. diff --git a/packages/components/tooltip/stories/Tooltip.stories.tsx b/packages/components/tooltip/stories/Tooltip.stories.tsx index 7f9091b6c8..0c4e4ddd1a 100644 --- a/packages/components/tooltip/stories/Tooltip.stories.tsx +++ b/packages/components/tooltip/stories/Tooltip.stories.tsx @@ -1,9 +1,10 @@ import React from 'react'; -import { SectionHeading } from '@contentful/f36-typography'; +import { Heading, Paragraph, SectionHeading } from '@contentful/f36-typography'; import { Tooltip } from '../src/Tooltip'; import { TextLink } from '@contentful/f36-text-link'; -import { Flex } from '@contentful/f36-core'; +import { Flex, Stack } from '@contentful/f36-core'; import { css } from 'emotion'; +import tokens from '@contentful/f36-tokens'; export default { title: 'Components/Tooltip', @@ -223,12 +224,15 @@ export const WithReactElement = () => { -

I am a React Element

+ + + I am a Heading + + + I am a paragraph + } > From 2eeb77a32ae21e77d15b389f933a175e6d3cf3f7 Mon Sep 17 00:00:00 2001 From: denkristoffer Date: Mon, 18 Sep 2023 16:26:37 +0200 Subject: [PATCH 08/11] refactor: avoid Omit on union type --- packages/components/avatar/src/Avatar/Avatar.tsx | 11 +++++++++-- packages/components/tooltip/src/Tooltip.tsx | 10 +++++++--- packages/components/tooltip/src/index.ts | 6 +++++- 3 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/components/avatar/src/Avatar/Avatar.tsx b/packages/components/avatar/src/Avatar/Avatar.tsx index 77c1f56fcb..a5a8c3b2e8 100644 --- a/packages/components/avatar/src/Avatar/Avatar.tsx +++ b/packages/components/avatar/src/Avatar/Avatar.tsx @@ -3,7 +3,12 @@ import { cx } from 'emotion'; import { type CommonProps } from '@contentful/f36-core'; import { Image, type ImageProps } from '@contentful/f36-image'; -import { Tooltip, type TooltipProps } from '@contentful/f36-tooltip'; +import { + Tooltip, + type TooltipProps, + type TooltipInternalProps, + type WithEnhancedContent, +} from '@contentful/f36-tooltip'; import { convertSizeToPixels, getAvatarStyles } from './Avatar.styles'; import type { ColorVariant } from './utils'; @@ -20,7 +25,9 @@ export interface AvatarProps extends CommonProps { /** * A tooltipProps attribute used to conditionally render the tooltip around root element */ - tooltipProps?: Omit; + tooltipProps?: CommonProps & + WithEnhancedContent & + Omit; variant?: Variant; colorVariant?: ColorVariant; icon?: React.ReactElement; diff --git a/packages/components/tooltip/src/Tooltip.tsx b/packages/components/tooltip/src/Tooltip.tsx index 6f64b6d322..fbc54af410 100644 --- a/packages/components/tooltip/src/Tooltip.tsx +++ b/packages/components/tooltip/src/Tooltip.tsx @@ -19,7 +19,7 @@ import { getStyles } from './Tooltip.styles'; export type TooltipPlacement = Placement; -type WithEnhancedContent = +export type WithEnhancedContent = | { /** * Content of the tooltip @@ -41,7 +41,7 @@ type WithEnhancedContent = label?: string; }; -export type TooltipProps = CommonProps & { +export type TooltipInternalProps = { /** * Child nodes to be rendered in the component and that will show the tooltip when they are hovered */ @@ -113,7 +113,11 @@ export type TooltipProps = CommonProps & { * @default false */ isDisabled?: boolean; -} & WithEnhancedContent; +}; + +export type TooltipProps = CommonProps & + TooltipInternalProps & + WithEnhancedContent; export const Tooltip = ({ children, diff --git a/packages/components/tooltip/src/index.ts b/packages/components/tooltip/src/index.ts index 27667e0037..5b906fd912 100644 --- a/packages/components/tooltip/src/index.ts +++ b/packages/components/tooltip/src/index.ts @@ -1,2 +1,6 @@ export { Tooltip } from './Tooltip'; -export type { TooltipProps } from './Tooltip'; +export type { + TooltipInternalProps, + TooltipProps, + WithEnhancedContent, +} from './Tooltip'; From 7790837604d81696f56dca5848039cd408c2a496 Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Mon, 18 Sep 2023 16:42:30 +0200 Subject: [PATCH 09/11] fix: remove unused propertiers --- packages/components/tooltip/stories/Tooltip.stories.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/components/tooltip/stories/Tooltip.stories.tsx b/packages/components/tooltip/stories/Tooltip.stories.tsx index 0c4e4ddd1a..a2901bb94b 100644 --- a/packages/components/tooltip/stories/Tooltip.stories.tsx +++ b/packages/components/tooltip/stories/Tooltip.stories.tsx @@ -2,8 +2,7 @@ import React from 'react'; import { Heading, Paragraph, SectionHeading } from '@contentful/f36-typography'; import { Tooltip } from '../src/Tooltip'; import { TextLink } from '@contentful/f36-text-link'; -import { Flex, Stack } from '@contentful/f36-core'; -import { css } from 'emotion'; +import { Flex } from '@contentful/f36-core'; import tokens from '@contentful/f36-tokens'; export default { From 7f0ef9b707e5f9da9f638a40e275094d4c51d58f Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Mon, 18 Sep 2023 16:52:14 +0200 Subject: [PATCH 10/11] fix: remove unused type --- packages/components/avatar/src/Avatar/Avatar.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/components/avatar/src/Avatar/Avatar.tsx b/packages/components/avatar/src/Avatar/Avatar.tsx index a5a8c3b2e8..2b151737f1 100644 --- a/packages/components/avatar/src/Avatar/Avatar.tsx +++ b/packages/components/avatar/src/Avatar/Avatar.tsx @@ -5,7 +5,6 @@ import { type CommonProps } from '@contentful/f36-core'; import { Image, type ImageProps } from '@contentful/f36-image'; import { Tooltip, - type TooltipProps, type TooltipInternalProps, type WithEnhancedContent, } from '@contentful/f36-tooltip'; From a1f2b400b2096fd0d014b593122ca34f37969dfa Mon Sep 17 00:00:00 2001 From: Miguel Crespo Date: Mon, 18 Sep 2023 17:30:18 +0200 Subject: [PATCH 11/11] chore: add changeset --- .changeset/breezy-geese-hug.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/breezy-geese-hug.md diff --git a/.changeset/breezy-geese-hug.md b/.changeset/breezy-geese-hug.md new file mode 100644 index 0000000000..21a71346c7 --- /dev/null +++ b/.changeset/breezy-geese-hug.md @@ -0,0 +1,6 @@ +--- +'@contentful/f36-avatar': minor +'@contentful/f36-tooltip': minor +--- + +Tooltips can now render ReactElement as their Content