From 9cf506ca1578f7197103296990e1b0d6e73ebbe8 Mon Sep 17 00:00:00 2001 From: dennisja Date: Sun, 17 May 2020 17:04:18 +0200 Subject: [PATCH 1/8] add unstyled prop to TooltipWithBounds --- .../src/tooltips/TooltipWithBounds.tsx | 18 ++++++++--------- .../test/TooltipWithBounds.test.tsx | 20 ++++++++++++++++++- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx b/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx index af32357bf..dbb90b4b2 100644 --- a/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx +++ b/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx @@ -1,7 +1,7 @@ import React from 'react'; import { withBoundingRects } from '@vx/bounds'; -import Tooltip from './Tooltip'; +import Tooltip, { TooltipProps, defaultStyles } from './Tooltip'; type RectShape = { top: number; @@ -18,14 +18,6 @@ type WithBoundingRectsProps = { parentRect?: RectShape; }; -type TooltipProps = { - left?: number; - top?: number; - className?: string; - style?: React.CSSProperties; - children?: React.ReactNode; -}; - type Props = { offsetLeft?: number; offsetTop?: number; @@ -42,6 +34,7 @@ function TooltipWithBounds({ getRects, children, style, + unstyled, ...otherProps }: Props) { let left = initialLeft; @@ -64,7 +57,12 @@ function TooltipWithBounds({ return ( {children} diff --git a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx index 217bf115d..795da415b 100644 --- a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx +++ b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx @@ -1,7 +1,25 @@ -import { TooltipWithBounds } from '../src'; +import React from 'react'; +import { mount } from 'enzyme'; +import { TooltipWithBounds, defaultStyles, Tooltip } from '../src'; describe('', () => { test('it should be defined', () => { expect(TooltipWithBounds).toBeDefined(); }); + + it('should render the Tooltip with default styles by default', () => { + const wrapper = mount(Hello); + const styles = wrapper.find(Tooltip).prop('style'); + Object.entries(defaultStyles).forEach(([key, value]) => { + expect(styles[key]).toBe(value); + }); + }); + + it('should render the tooltip without default styles if unstyled is set to true', () => { + const wrapper = mount(Hello); + const styles = wrapper.find(Tooltip).prop('style'); + Object.keys(defaultStyles).forEach(key => { + expect(styles[key]).toBeUndefined(); + }); + }); }); From 6c382e9ffa0c374c8eeb9d9de2a5e9ffd0012176 Mon Sep 17 00:00:00 2001 From: dennisja Date: Sun, 17 May 2020 17:08:05 +0200 Subject: [PATCH 2/8] update docs --- packages/vx-tooltip/Readme.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vx-tooltip/Readme.md b/packages/vx-tooltip/Readme.md index db4e4cd64..cd076a352 100644 --- a/packages/vx-tooltip/Readme.md +++ b/packages/vx-tooltip/Readme.md @@ -69,7 +69,7 @@ accepts the following props, and will spread any additional props on the tooltip | className | string | -- | Adds a class (in addition to `vx-tooltip-portal`) to the tooltip container | | style | object | -- | Sets / overrides any styles on the tooltip container (including top and left) | | children | node | -- | Sets the children of the tooltip, i.e., the actual content | -| unstyled | bool | true | Whether the tooltip use styles from the style prop or not | +| unstyled | bool | true | Whether the tooltip should use styles from the style prop or not | #### TooltipWithBounds @@ -86,6 +86,7 @@ component (i.e., ...restProps): | offsetRight | number | 10 | Vertical offset of the tooltip from the passed `top` value, functions as a vertical padding. | | style | object | -- | Sets / overrides any styles on the tooltip container (including top and left) | | children | node | -- | Sets the children of the tooltip, i.e., the actual content | +| unstyled | bool | true | Whether the tooltip should render with default styles or not | Note that this component is positioned using a `transform`, so overriding `left` and `top` via styles may have no effect. From 5a7360539def9cd2f12768a786269d03c75c99ef Mon Sep 17 00:00:00 2001 From: dennisja Date: Sun, 17 May 2020 17:29:13 +0200 Subject: [PATCH 3/8] make styles non-null to fix the build --- packages/vx-tooltip/test/TooltipWithBounds.test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx index 795da415b..f76bd85c4 100644 --- a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx +++ b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx @@ -9,7 +9,7 @@ describe('', () => { it('should render the Tooltip with default styles by default', () => { const wrapper = mount(Hello); - const styles = wrapper.find(Tooltip).prop('style'); + const styles = wrapper.find(Tooltip).props().style!; Object.entries(defaultStyles).forEach(([key, value]) => { expect(styles[key]).toBe(value); }); @@ -17,7 +17,7 @@ describe('', () => { it('should render the tooltip without default styles if unstyled is set to true', () => { const wrapper = mount(Hello); - const styles = wrapper.find(Tooltip).prop('style'); + const styles = wrapper.find(Tooltip).props().style!; Object.keys(defaultStyles).forEach(key => { expect(styles[key]).toBeUndefined(); }); From 8382202defdae9b9cd39fb18215862e664efae40 Mon Sep 17 00:00:00 2001 From: dennisja Date: Sun, 17 May 2020 17:51:23 +0200 Subject: [PATCH 4/8] add types to the callbacks to fix the build --- packages/vx-tooltip/test/TooltipWithBounds.test.tsx | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx index f76bd85c4..6e29a63c3 100644 --- a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx +++ b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx @@ -2,6 +2,9 @@ import React from 'react'; import { mount } from 'enzyme'; import { TooltipWithBounds, defaultStyles, Tooltip } from '../src'; +type CSSPropertiesKey = keyof React.CSSProperties; +type CSSPropertiesValue = React.CSSProperties[CSSPropertiesKey]; + describe('', () => { test('it should be defined', () => { expect(TooltipWithBounds).toBeDefined(); @@ -10,15 +13,17 @@ describe('', () => { it('should render the Tooltip with default styles by default', () => { const wrapper = mount(Hello); const styles = wrapper.find(Tooltip).props().style!; - Object.entries(defaultStyles).forEach(([key, value]) => { - expect(styles[key]).toBe(value); - }); + Object.entries(defaultStyles).forEach( + ([key, value]: [CSSPropertiesKey, CSSPropertiesValue]) => { + expect(styles[key]).toBe(value); + }, + ); }); it('should render the tooltip without default styles if unstyled is set to true', () => { const wrapper = mount(Hello); const styles = wrapper.find(Tooltip).props().style!; - Object.keys(defaultStyles).forEach(key => { + Object.keys(defaultStyles).forEach((key: CSSPropertiesKey) => { expect(styles[key]).toBeUndefined(); }); }); From 806fbf2626189d7edb73db3ac0bf49523c3e75d5 Mon Sep 17 00:00:00 2001 From: dennisja Date: Sun, 17 May 2020 18:01:44 +0200 Subject: [PATCH 5/8] make the styles object in test any to fix the build --- .../vx-tooltip/test/TooltipWithBounds.test.tsx | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx index 6e29a63c3..9906de765 100644 --- a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx +++ b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx @@ -2,9 +2,6 @@ import React from 'react'; import { mount } from 'enzyme'; import { TooltipWithBounds, defaultStyles, Tooltip } from '../src'; -type CSSPropertiesKey = keyof React.CSSProperties; -type CSSPropertiesValue = React.CSSProperties[CSSPropertiesKey]; - describe('', () => { test('it should be defined', () => { expect(TooltipWithBounds).toBeDefined(); @@ -12,18 +9,16 @@ describe('', () => { it('should render the Tooltip with default styles by default', () => { const wrapper = mount(Hello); - const styles = wrapper.find(Tooltip).props().style!; - Object.entries(defaultStyles).forEach( - ([key, value]: [CSSPropertiesKey, CSSPropertiesValue]) => { - expect(styles[key]).toBe(value); - }, - ); + const styles = wrapper.find(Tooltip).props().style as any; + Object.entries(defaultStyles).forEach(([key, value]) => { + expect(styles[key]).toBe(value); + }); }); it('should render the tooltip without default styles if unstyled is set to true', () => { const wrapper = mount(Hello); - const styles = wrapper.find(Tooltip).props().style!; - Object.keys(defaultStyles).forEach((key: CSSPropertiesKey) => { + const styles = wrapper.find(Tooltip).props().style as any; + Object.keys(defaultStyles).forEach(key => { expect(styles[key]).toBeUndefined(); }); }); From 70be7a4aa1486b79b7c9c6dab300a9e20b5ba694 Mon Sep 17 00:00:00 2001 From: dennisja Date: Tue, 19 May 2020 21:54:02 +0200 Subject: [PATCH 6/8] make TooltipWithBounds API match that of Tooltip --- packages/vx-tooltip/Readme.md | 2 +- packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/vx-tooltip/Readme.md b/packages/vx-tooltip/Readme.md index cd076a352..d8211a5ee 100644 --- a/packages/vx-tooltip/Readme.md +++ b/packages/vx-tooltip/Readme.md @@ -86,7 +86,7 @@ component (i.e., ...restProps): | offsetRight | number | 10 | Vertical offset of the tooltip from the passed `top` value, functions as a vertical padding. | | style | object | -- | Sets / overrides any styles on the tooltip container (including top and left) | | children | node | -- | Sets the children of the tooltip, i.e., the actual content | -| unstyled | bool | true | Whether the tooltip should render with default styles or not | +| unstyled | bool | true | Whether the tooltip should use styles from the style prop or not | Note that this component is positioned using a `transform`, so overriding `left` and `top` via styles may have no effect. diff --git a/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx b/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx index dbb90b4b2..866fc13b3 100644 --- a/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx +++ b/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx @@ -33,7 +33,7 @@ function TooltipWithBounds({ parentRect, getRects, children, - style, + style = defaultStyles, unstyled, ...otherProps }: Props) { @@ -60,8 +60,7 @@ function TooltipWithBounds({ style={{ top: 0, transform: `translate(${left}px, ${top}px)`, - ...style, - ...(!unstyled && defaultStyles), + ...(!unstyled && style), }} {...otherProps} > From 0e404478585faeea6aae23f8aa8d985228f9f448 Mon Sep 17 00:00:00 2001 From: dennisja Date: Tue, 19 May 2020 21:58:12 +0200 Subject: [PATCH 7/8] add unstyled default value in TooltipWithBounds props --- packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx b/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx index 866fc13b3..c319b33ce 100644 --- a/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx +++ b/packages/vx-tooltip/src/tooltips/TooltipWithBounds.tsx @@ -34,7 +34,7 @@ function TooltipWithBounds({ getRects, children, style = defaultStyles, - unstyled, + unstyled = false, ...otherProps }: Props) { let left = initialLeft; @@ -62,6 +62,7 @@ function TooltipWithBounds({ transform: `translate(${left}px, ${top}px)`, ...(!unstyled && style), }} + unstyled={unstyled} {...otherProps} > {children} From 619a0036ef80230a102577fbc63057e8e0a6b16f Mon Sep 17 00:00:00 2001 From: dennisja Date: Fri, 29 May 2020 08:47:44 +0200 Subject: [PATCH 8/8] use shallow in tests --- .../vx-tooltip/test/TooltipWithBounds.test.tsx | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx index 9906de765..8db249323 100644 --- a/packages/vx-tooltip/test/TooltipWithBounds.test.tsx +++ b/packages/vx-tooltip/test/TooltipWithBounds.test.tsx @@ -1,6 +1,6 @@ import React from 'react'; -import { mount } from 'enzyme'; -import { TooltipWithBounds, defaultStyles, Tooltip } from '../src'; +import { shallow } from 'enzyme'; +import { TooltipWithBounds, defaultStyles } from '../src'; describe('', () => { test('it should be defined', () => { @@ -8,16 +8,20 @@ describe('', () => { }); it('should render the Tooltip with default styles by default', () => { - const wrapper = mount(Hello); - const styles = wrapper.find(Tooltip).props().style as any; + const wrapper = shallow(Hello, { + disableLifecycleMethods: true, + }).dive(); + const styles = wrapper.find('Tooltip').props().style as any; Object.entries(defaultStyles).forEach(([key, value]) => { expect(styles[key]).toBe(value); }); }); it('should render the tooltip without default styles if unstyled is set to true', () => { - const wrapper = mount(Hello); - const styles = wrapper.find(Tooltip).props().style as any; + const wrapper = shallow(Hello, { + disableLifecycleMethods: true, + }).dive(); + const styles = wrapper.find('Tooltip').props().style as any; Object.keys(defaultStyles).forEach(key => { expect(styles[key]).toBeUndefined(); });