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.

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

Merged
merged 7 commits into from
Dec 22, 2022
+31 −78
Merged

Minor bug fixes and docs improvements #873

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9e9f092
fix: check `children` when showing tooltip
gabrieljablonski Dec 21, 2022
108a9f8
fix: interaction between `content` and `html`
gabrieljablonski Dec 21, 2022
60eceb5
docs: minor documentantion improvements
gabrieljablonski Dec 21, 2022
af664b9
fix: properly update `content` when it is empty
gabrieljablonski Dec 22, 2022
27e4f49
docs: re-add "Content" example page
gabrieljablonski Dec 22, 2022
b1d25f6
docs: remove section from "Events" example page
gabrieljablonski Dec 22, 2022
8002c99
chore: bump project version and docs package
gabrieljablonski Dec 22, 2022
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
80 changes: 0 additions & 80 deletions docs/docs/examples/content.mdx
View file
Open in desktop

This file was deleted.

16 changes: 8 additions & 8 deletions docs/docs/examples/delay.mdx
View file
Open in desktop
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
25 changes: 0 additions & 25 deletions docs/docs/examples/multiline.mdx
View file
Open in desktop
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
View file
Open in desktop
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
4 changes: 3 additions & 1 deletion src/components/Tooltip/Tooltip.tsx
View file
Open in desktop
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
6 changes: 4 additions & 2 deletions src/components/TooltipController/TooltipController.tsx
View file
Open in desktop
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 @@ -111,9 +110,12 @@ const TooltipController = ({

useEffect(() => {
if (content) {
setIsHtmlContent(false)
setTooltipContent(content)
}
if (html) {
// html will always 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