From 9bb9af26b12bab5ea3be6a0c8403e2de957f58ab Mon Sep 17 00:00:00 2001 From: AnnMarieW Date: Thu, 16 Sep 2021 12:09:18 -0700 Subject: [PATCH 1/3] Allow dict ids as target id for Tooltip --- src/components/tooltip/Tooltip.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/components/tooltip/Tooltip.js b/src/components/tooltip/Tooltip.js index 29f09644d..5abd786ef 100644 --- a/src/components/tooltip/Tooltip.js +++ b/src/components/tooltip/Tooltip.js @@ -11,6 +11,19 @@ import Overlay from '../../private/Overlay'; * Simply add the Tooltip to you layout, and give it a target (id of a * component to which the tooltip should be attached) */ + +// stringifies object ids used in pattern matching callbacks +const stringifyId = id => { + if (typeof id !== 'object') { + return id; + } + const stringifyVal = v => (v && v.wild) || JSON.stringify(v); + const parts = Object.keys(id) + .sort() + .map(k => JSON.stringify(k) + ':' + stringifyVal(id[k])); + return '{' + parts.join(',') + '}'; +}; + const Tooltip = props => { const { id, @@ -19,6 +32,7 @@ const Tooltip = props => { className, class_name, style, + target, ...otherProps } = props; @@ -29,6 +43,7 @@ const Tooltip = props => { } {...omit(['setProps'], otherProps)} trigger="hover focus" + target={stringifyId(target)} > {children} From 95d3be25ea737d5e1a08ceabf1e887b85f387067 Mon Sep 17 00:00:00 2001 From: AnnMarieW Date: Thu, 16 Sep 2021 12:33:34 -0700 Subject: [PATCH 2/3] Included Popover too --- src/components/popover/Popover.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/components/popover/Popover.js b/src/components/popover/Popover.js index 84e6562b8..bcc6abc4b 100644 --- a/src/components/popover/Popover.js +++ b/src/components/popover/Popover.js @@ -12,6 +12,19 @@ import Overlay from '../../private/Overlay'; * Use the `PopoverHeader` and `PopoverBody` components to control the layout * of the children. */ + +// stringifies object ids used in pattern matching callbacks +const stringifyId = id => { + if (typeof id !== 'object') { + return id; + } + const stringifyVal = v => (v && v.wild) || JSON.stringify(v); + const parts = Object.keys(id) + .sort() + .map(k => JSON.stringify(k) + ':' + stringifyVal(id[k])); + return '{' + parts.join(',') + '}'; +}; + const Popover = props => { const { children, @@ -21,6 +34,7 @@ const Popover = props => { class_name, style, id, + target, ...otherProps } = props; @@ -30,6 +44,7 @@ const Popover = props => { (loading_state && loading_state.is_loading) || undefined } defaultShow={is_open} + target={stringifyId(target)} {...otherProps} > Date: Thu, 16 Sep 2021 15:00:55 -0700 Subject: [PATCH 3/3] update based on review --- docs/content/docs/faq.md | 52 -------------------- src/components/popover/Popover.js | 16 +------ src/components/tooltip/Tooltip.js | 16 +------ src/private/Overlay.js | 18 ++++++- src/private/__tests__/Overlay.test.js | 69 +++++++++++++++++++++++++++ 5 files changed, 87 insertions(+), 84 deletions(-) create mode 100644 src/private/__tests__/Overlay.test.js diff --git a/docs/content/docs/faq.md b/docs/content/docs/faq.md index 2c7f1f7d1..95426c831 100644 --- a/docs/content/docs/faq.md +++ b/docs/content/docs/faq.md @@ -6,58 +6,6 @@ title: FAQ This page contains various tips and tricks and answers to frequently asked questions about _dash-bootstrap-components_. If you think something is missing, please submit an [issue][issue] on the GitHub issue tracker. -### How do I use `Tooltip` or `Popover` with pattern-matching callbacks? - -Dash 1.11.0 added support for [pattern matching callbacks](https://dash.plotly.com/pattern-matching-callbacks) which allows you to write callbacks that can update an arbitrary or dynamic number of Dash components. To enable this, the `id` of a Dash component can now be a Python dictionary, and the callback is triggered based on a matching rule with one or more of the keys in this dictionary. - -However, it is not possible to use a dictionary as the `target` of the `Popover` or `Tooltip` components. The reason for this is explained below. To get around the problem, the best thing to do is to wrap your dynamically created components with a `html.Div` element or similar, and use a string `id` for the wrapper which you then use as the target for the `Tooltip` or `Popover`. For example this example from the Dash documentation - -```python -@app.callback( - Output('dropdown-container', 'children'), - Input('add-filter', 'n_clicks'), - State('dropdown-container', 'children')) -def display_dropdowns(n_clicks, children): - new_dropdown = dcc.Dropdown( - id={ - 'type': 'filter-dropdown', - 'index': n_clicks - }, - options=[{'label': i, 'value': i} for i in ['NYC', 'MTL', 'LA', 'TOKYO']] - ) - children.append(new_dropdown) - return children -``` - -might become the following - -```python -@app.callback( - Output('dropdown-container', 'children'), - Input('add-filter', 'n_clicks'), - State('dropdown-container', 'children')) -def display_dropdowns(n_clicks, children): - new_dropdown = html.Div( - dcc.Dropdown( - id={ - 'type': 'filter-dropdown', - 'index': n_clicks - }, - options=[{'label': i, 'value': i} for i in ['NYC', 'MTL', 'LA', 'TOKYO']] - ), - id=f"dropdown-wrapper-{n_clicks}" - ) - new_tooltip = dbc.Tooltip( - f"This is dropdown number {n_clicks}", - target=f"dropdown-wrapper-{n_clicks}", - ) - children.append(new_dropdown) - children.append(new_tooltip) - return children -``` - -The reason `Popover` and `Tooltip` can't support the dictionary-based `id` is that under the hood these components are searching for the `id` using a function called `querySelectorAll` implemented as part of the standard Web APIs. This function can only search for a valid CSS selector string, which is restricted more or less to alphanumeric characters plus hyphens and underscores. Dash serialises dictionary ids as JSON, which contains characters like `{` and `}` that are invalid in CSS selectors. The above workaround avoids this issue. - ### How do I scale the viewport on mobile devices? When building responsive layouts it is typical to have something like the following in your HTML template diff --git a/src/components/popover/Popover.js b/src/components/popover/Popover.js index bcc6abc4b..a843dcb68 100644 --- a/src/components/popover/Popover.js +++ b/src/components/popover/Popover.js @@ -13,18 +13,6 @@ import Overlay from '../../private/Overlay'; * of the children. */ -// stringifies object ids used in pattern matching callbacks -const stringifyId = id => { - if (typeof id !== 'object') { - return id; - } - const stringifyVal = v => (v && v.wild) || JSON.stringify(v); - const parts = Object.keys(id) - .sort() - .map(k => JSON.stringify(k) + ':' + stringifyVal(id[k])); - return '{' + parts.join(',') + '}'; -}; - const Popover = props => { const { children, @@ -34,7 +22,6 @@ const Popover = props => { class_name, style, id, - target, ...otherProps } = props; @@ -44,7 +31,6 @@ const Popover = props => { (loading_state && loading_state.is_loading) || undefined } defaultShow={is_open} - target={stringifyId(target)} {...otherProps} > { - if (typeof id !== 'object') { - return id; - } - const stringifyVal = v => (v && v.wild) || JSON.stringify(v); - const parts = Object.keys(id) - .sort() - .map(k => JSON.stringify(k) + ':' + stringifyVal(id[k])); - return '{' + parts.join(',') + '}'; -}; - const Tooltip = props => { const { id, @@ -32,7 +20,6 @@ const Tooltip = props => { className, class_name, style, - target, ...otherProps } = props; @@ -43,7 +30,6 @@ const Tooltip = props => { } {...omit(['setProps'], otherProps)} trigger="hover focus" - target={stringifyId(target)} > {children} @@ -96,7 +82,7 @@ Tooltip.propTypes = { /** * The id of the element to attach the tooltip to */ - target: PropTypes.string, + target: PropTypes.oneOfType([PropTypes.string, PropTypes.object]), /** * How to place the tooltip. diff --git a/src/private/Overlay.js b/src/private/Overlay.js index 949d04e48..f71fb2907 100644 --- a/src/private/Overlay.js +++ b/src/private/Overlay.js @@ -19,6 +19,18 @@ function useStateRef(initialValue) { return [value, setValue, ref]; } +// stringifies object ids used in pattern matching callbacks +const stringifyId = id => { + if (typeof id !== 'object') { + return id; + } + const stringifyVal = v => (v && v.wild) || JSON.stringify(v); + const parts = Object.keys(id) + .sort() + .map(k => JSON.stringify(k) + ':' + stringifyVal(id[k])); + return '{' + parts.join(',') + '}'; +}; + const Overlay = ({ children, target, @@ -40,6 +52,8 @@ const Overlay = ({ const triggers = typeof trigger === 'string' ? trigger.split(' ') : []; + const targetStr = stringifyId(target); + const hide = () => { if (isOpenRef.current) { hideTimeout.current = clearTimeout(hideTimeout.current); @@ -133,9 +147,9 @@ const Overlay = ({ }, [defaultShow]); useEffect(() => { - targetRef.current = document.getElementById(target); + targetRef.current = document.getElementById(targetStr); addEventListeners(targetRef.current); - }, [target]); + }, [targetStr]); return ( diff --git a/src/private/__tests__/Overlay.test.js b/src/private/__tests__/Overlay.test.js new file mode 100644 index 000000000..1cac89ed1 --- /dev/null +++ b/src/private/__tests__/Overlay.test.js @@ -0,0 +1,69 @@ +import React from 'react'; +import {act, fireEvent, render} from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import Tooltip from '../../components/tooltip/Tooltip'; +jest.useFakeTimers(); + +describe('Tooltip with dict id', () => { + // this is just a little hack to silence a warning that we'll get until we + // upgrade to 16.9. See also: https://github.com/facebook/react/pull/14853 + const originalError = console.error; + beforeAll(() => { + console.error = (...args) => { + if (/Warning.*not wrapped in act/.test(args[0])) { + return; + } + originalError.call(console, ...args); + }; + }); + + afterAll(() => { + console.error = originalError; + }); + + let div; + beforeAll(() => { + div = document.createElement('div'); + div.setAttribute('id', '{"index":1,"type":"target"}'); + }); + + test('renders nothing by default', () => { + render( + Test content, + { + container: document.body.appendChild(div) + } + ); + + expect(document.body.querySelector('.tooltip')).toBe(null); + }); + + test('renders a div with class "tooltip"', () => { + render(, { + container: document.body.appendChild(div) + }); + + fireEvent.mouseOver(div); + act(() => jest.runAllTimers()); + expect(document.body.querySelector('.tooltip')).not.toBe(null); + + fireEvent.mouseLeave(div); + act(() => jest.runAllTimers()); + expect(document.body.querySelector('.tooltip')).toBe(null); + }); + + test('renders its content', () => { + render( + Tooltip content, + { + container: document.body.appendChild(div) + } + ); + + fireEvent.mouseOver(div); + act(() => jest.runAllTimers()); + expect(document.body.querySelector('.tooltip')).toHaveTextContent( + 'Tooltip content' + ); + }); +});