Skip to content

Minor bug fixes and docs improvements #873

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

Merged
merged 7 commits into from
Dec 22, 2022
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
21 changes: 8 additions & 13 deletions docs/docs/examples/content.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -40,14 +40,14 @@ import { Tooltip } from 'react-tooltip';
import 'react-tooltip/dist/react-tooltip.css';

<a id="tooltip-anchor"> ◕‿‿◕ </a>
<Tooltip anchorId="tooltip-anchor" content="Hello<br /><s>multiline</s><br /><b>HTML</b><br />tooltip" place="left" />
<Tooltip anchorId="tooltip-anchor" content="hello world!" place="left" />
```

<div style={{ display: 'flex', columnGap: '16px', justifyContent: 'center', paddingTop: '36px' }}>
<TooltipAnchor id="tooltip-anchor">◕‿‿◕</TooltipAnchor>
<Tooltip
anchorId="tooltip-anchor"
content="Hello<br /><s>multiline</s><br /><b>HTML</b><br />tooltip"
content="hello world!"
/>
</div>

Expand All @@ -58,23 +58,18 @@ import { Tooltip } from 'react-tooltip';
import 'react-tooltip/dist/react-tooltip.css';

<a
id="tooltip-anchor-data-html"
data-tooltip-content="<div><h1>Tooltip</h1><ul><li>Lorem</li><li>Ipsum</li></ul></div>"
id="tooltip-anchor-data-tooltip-content"
data-tooltip-content="hello world!"
> ◕‿‿◕ </a>
<Tooltip anchorId="tooltip-anchor-data-html" />
<Tooltip anchorId="tooltip-anchor-data-tooltip-content" />
```

<div style={{ display: 'flex', columnGap: '16px', justifyContent: 'center', paddingTop: '36px' }}>
<TooltipAnchor
id="tooltip-anchor-html"
data-tooltip-content="<div><h1>Tooltip</h1><ul><li>Lorem</li><li>Ipsum</li></ul></div>"
id="tooltip-anchor-data-tooltip-content"
data-tooltip-content="hello world!"
>
◕‿‿◕
</TooltipAnchor>
<Tooltip anchorId="tooltip-anchor-html" />
<Tooltip anchorId="tooltip-anchor-data-tooltip-content" />
</div>

#### Observations

- When using `data-tooltip-content` the HTML tags works by default like [HTML](./html.mdx) param
- When using prop `content` the HTML doesn't work by default, use prop `html` instead of this
16 changes: 8 additions & 8 deletions docs/docs/examples/delay.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -41,17 +41,17 @@ import 'react-tooltip/dist/react-tooltip.css';

<a
id="tooltip-anchor-hide"
data-tooltip-content="hover on me will keep the tooltip"
data-tooltip-delay-hide={3000}
data-tooltip-content="It will take me 1000ms to close"
data-tooltip-delay-hide={1000}
> ◕‿‿◕ </a>
<Tooltip anchorId="tooltip-anchor-hide" />
```

<div style={{ display: 'flex', columnGap: '16px', justifyContent: 'center', paddingTop: '36px' }}>
<TooltipAnchor
id="tooltip-anchor-hide"
data-tooltip-content="hover on me will keep the tooltip"
data-tooltip-delay-hide={3000}
data-tooltip-content="It will take me 1000ms to close"
data-tooltip-delay-hide={1000}
>
◕‿‿◕
</TooltipAnchor>
Expand All @@ -66,17 +66,17 @@ import 'react-tooltip/dist/react-tooltip.css';

<a
id="tooltip-anchor-show"
data-tooltip-content="hover on me will keep the tooltip"
data-tooltip-delay-show={3000}
data-tooltip-content="It took me 1000ms to open"
data-tooltip-delay-show={1000}
> ◕‿‿◕ </a>
<Tooltip anchorId="tooltip-anchor-show" />
```

<div style={{ display: 'flex', columnGap: '16px', justifyContent: 'center', paddingTop: '36px' }}>
<TooltipAnchor
id="tooltip-anchor-show"
data-tooltip-content="hover on me will keep the tooltip"
data-tooltip-delay-show={3000}
data-tooltip-content="It took me 1000ms to open"
data-tooltip-delay-show={1000}
>
◕‿‿◕
</TooltipAnchor>
Expand Down
19 changes: 0 additions & 19 deletions docs/docs/examples/events.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -64,22 +64,3 @@ import 'react-tooltip/dist/react-tooltip.css';
<TooltipAnchor id="tooltip-anchor-click">◕‿‿◕</TooltipAnchor>
<Tooltip anchorId="tooltip-anchor-click" content="Hello World!" events={['click']} />
</div>

### Using `hover` and `click`

```jsx
import { Tooltip } from 'react-tooltip';
import 'react-tooltip/dist/react-tooltip.css';

<a id="tooltip-anchor-hover-click"> ◕‿‿◕ </a>
<Tooltip anchorId="tooltip-anchor-hover-click" content="Hello World!" events={['hover', 'click']} />
```

<div style={{ display: 'flex', columnGap: '16px', justifyContent: 'center', paddingTop: '36px' }}>
<TooltipAnchor id="tooltip-anchor-hover-click">◕‿‿◕</TooltipAnchor>
<Tooltip
anchorId="tooltip-anchor-hover-click"
content="Hello World!"
events={['hover', 'click']}
/>
</div>
25 changes: 0 additions & 25 deletions docs/docs/examples/multiline.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -48,26 +48,6 @@ import 'react-tooltip/dist/react-tooltip.css';
<Tooltip anchorId="tooltip-anchor" html="Hello<br />multiline<br />tooltip" />
</div>

### Using `data-tooltip-content`

```jsx
import { Tooltip } from 'react-tooltip';
import 'react-tooltip/dist/react-tooltip.css';

<a id="tooltip-anchor-data-content" data-tooltip-content="Hello<br />multiline<br />tooltip"> ◕‿‿◕ </a>
<Tooltip anchorId="tooltip-anchor-data-content" />
```

<div style={{ display: 'flex', columnGap: '16px', justifyContent: 'center', paddingTop: '36px' }}>
<TooltipAnchor
id="tooltip-anchor-data-content"
data-tooltip-content="Hello<br />multiline<br />tooltip"
>
◕‿‿◕
</TooltipAnchor>
<Tooltip anchorId="tooltip-anchor-data-content" />
</div>

### Using `data-tooltip-html`

```jsx
Expand All @@ -84,8 +64,3 @@ import 'react-tooltip/dist/react-tooltip.css';
</TooltipAnchor>
<Tooltip anchorId="tooltip-anchor-html" />
</div>

#### Observations

- When using `data-tooltip-content` the `<br />` works by default
- When using prop `content` the `<br />` doesn't work by default, use prop `html` instead of this
2 changes: 1 addition & 1 deletion docs/docs/getting-started.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ import { Tooltip } from 'react-tooltip'

### Styling

If you want a styled tooltip, don't forget to add the style file `import 'react-tooltip/dist/react-tooltip.css`.
If you want a styled tooltip, don't forget to add the style file `import 'react-tooltip/dist/react-tooltip.css'`.

Basic styling can be done by overriding the following css variables.

Expand Down
2 changes: 1 addition & 1 deletion docs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
"raw-loader": "^4.0.2",
"react": "18.2.0",
"react-dom": "18.2.0",
"react-tooltip": "5.3.0"
"react-tooltip": "5.3.1"
},
"devDependencies": {
"@docusaurus/module-type-aliases": "2.2.0",
Expand Down
8 changes: 4 additions & 4 deletions docs/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6112,10 +6112,10 @@ react-textarea-autosize@^8.3.2:
use-composed-ref "^1.3.0"
use-latest "^1.2.1"

[email protected].0:
version "5.3.0"
resolved "https://registry.yarnpkg.com/react-tooltip/-/react-tooltip-5.3.0.tgz#cba9d40396bf7c15c5d748425f76ec5c3e69b213"
integrity sha512-dE702vnPYYUPDWeFHCMzyCbJ3Ca3c160p1EQvumacTl19a0RjVJ4KHBT4XCJ1FVPLKjI6xJR0+RuSAzWyUkGow==
[email protected].1:
version "5.3.1"
resolved "https://registry.yarnpkg.com/react-tooltip/-/react-tooltip-5.3.1.tgz#b2bb849384c5e1fa182460efb3ea4f8ebad69fca"
integrity sha512-lBX24Xvs/ZGwq+3waku9FHNAqIIjjlq+Hlk6R+mpbmeGM6LQ9zUfMKlPtWynZg50ZZ1/3ndlt8YtCEPNPCT5gw==
dependencies:
"@floating-ui/dom" "^1.0.4"
classnames "^2.3.2"
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "react-tooltip",
"version": "5.3.0",
"version": "5.3.1",
"description": "react tooltip component",
"scripts": {
"dev": "node ./cli.js --env=development && node --max_old_space_size=2048 ./node_modules/rollup/dist/bin/rollup -c rollup.config.dev.js --watch",
Expand Down
4 changes: 3 additions & 1 deletion src/components/Tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -291,12 +291,14 @@ const Tooltip = ({
}
}, [])

const hasContentOrChildren = Boolean(content || children)

return (
<WrapperElement
id={id}
role="tooltip"
className={classNames(styles['tooltip'], styles[variant], className, {
[styles['show']]: content && !calculatingPosition && (isOpen || show),
[styles['show']]: hasContentOrChildren && !calculatingPosition && (isOpen || show),
[styles['fixed']]: positionStrategy === 'fixed',
})}
style={{ ...externalStyles, ...inlineStyles }}
Expand Down
10 changes: 5 additions & 5 deletions src/components/TooltipController/TooltipController.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,11 +68,10 @@ const TooltipController = ({
setTooltipPlace((value as PlacesType) ?? place)
},
content: (value) => {
setIsHtmlContent(false)
setTooltipContent(value ?? content)
},
html: (value) => {
setIsHtmlContent(!!value)
setIsHtmlContent(Boolean(value ?? html))
setTooltipContent(value ?? html ?? content)
},
variant: (value) => {
Expand Down Expand Up @@ -110,10 +109,11 @@ const TooltipController = ({
}

useEffect(() => {
if (content) {
setTooltipContent(content)
}
setIsHtmlContent(false)
setTooltipContent(content)
if (html) {
// html will take precedence
setIsHtmlContent(true)
setTooltipContent(html)
Copy link
Member

Choose a reason for hiding this comment

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

I have the feeling that we will spend resources (a bit) doing the:

setIsHtmlContent(false)

and after that:

setIsHtmlContent(true)

In this case, I prefer not to do the setIsHtmlContent(false) if we will do setIsHtmlContent(true) two lines below.

Is this necessary in this order? or can we add a validation to do one or other?

Copy link
Member Author

Choose a reason for hiding this comment

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

The validation would only serve to make the code clearer (which I think is unnecessary, since it's simple enough).

React 18 batches useState() updates, so the state is effectively only updated once. This source is a bit outdated, but I'm pretty sure that still holds true.

We can come back to this in the future if we ever decide to improve the content/html props (maybe there shouldn't be a single state variable - tooltipContent - that holds the value for the one being actually used).

Copy link
Member

Choose a reason for hiding this comment

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

oh, interesting!

ok

}
}, [content, html])
Expand Down