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.

Sign up for GitHub

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

Jump to bottom

Tooltip content should be wrapped in <div> instead of <p> #17344

Closed

zhigzhen opened this issue Mar 10, 2021 · 16 comments · Fixed by #17593

Assignees

Labels

Component: Tooltip Fluent UI react (v8) Needs: Investigation Status: Fixed

Contributor

zhigzhen commented Mar 10, 2021 •

edited

Loading

Environment Information

Package version(s): 7.155.1
Browser and OS versions: N/A

Actual behavior:

If a JSX object with a <div> other than a string is passed into the content field of a TooltipHost / Tooltip, the browser will complain with the following error:

Warning: validateDOMnesting(...): <div> cannot appear as a descendant of <p>...

Expected behavior:

The browser should not complain because the content should be wrapped inside of <div> instead of <p>. This change can be made on line 71 here: https://github.com/microsoft/fluentui/blob/master/packages/react/src/components/Tooltip/Tooltip.base.tsx

Another reason for this change, is that content field accepts either string or JSX.Element so it should account for nested objects within a <div> when a JSX.Element is provided.

source: https://github.com/microsoft/fluentui/blob/master/packages/react/src/components/Tooltip/Tooltip.types.ts line 33

Priorities and help requested:

Are you willing to submit a PR to fix? Yes

The text was updated successfully, but these errors were encountered:

msft-fluent-ui-bot added the Needs: Triage 🔍 label

msft-fluent-ui-bot assigned khmakoto

gouttierre added Component: Tooltip Fluent UI react (v8) Needs: Investigation and removed Needs: Triage 🔍 labels

Contributor

gouttierre commented Mar 15, 2021

@zhigzhen - Thanks for reaching out and submitting this issue. I see you are willing to submit a PR for this request - that is great - Would you mind sharing it with us?

@khmakoto - can you review the suggestion posted where the content should be wrapped inside of <div> instead of <p> on line 71 here: https://github.com/microsoft/fluentui/blob/master/packages/react/src/components/Tooltip/Tooltip.base.tsx

zhigzhen mentioned this issue

Wrap Tooltip content in <div> for non string content type #17593

Merged

2 tasks

msft-fluent-ui-bot added the Status: In PR label

msft-fluent-ui-bot closed this as completed in #17593

msft-fluent-ui-bot pushed a commit that referenced this issue


          Wrap Tooltip content in <div> for non string content type (#17593)

644df65

#### Pull request checklist

- [x] Addresses an existing issue: Fixes #17344
- [x] Include a change request file using $ yarn change

#### Description of changes

See the issue thread for more context. 

If a JSX object with a `<div>` other than a string is passed into the content field of a `TooltipHost` / `Tooltip`, the browser will complain with the following error:

> Warning: validateDOMnesting(...): `<div>` cannot appear as a descendant of `<p>`...

To deal with this error, we should only wrap the object in `<p>` if it is type string, otherwise we can wrap it in a `<div>`

msft-fluent-ui-bot added Status: Fixed and removed Status: In PR labels

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

Collaborator

msft-fluent-ui-bot commented May 18, 2021

🎉This issue was addressed in #17593, which has now been successfully released as @fluentui/[email protected].:tada:

Handy links:

microsoft locked as resolved and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.