Skip to content

Commit d9437ae

Browse files
committed
feat(ui-popover): make ui-popover backwards compatible
refs: INSTUI-2333 This commit makes ui-popover backwards compatible with the API from ui-overlays/Popover. This should allow consumers to upgade to ui-popover without any breaking changes. TEST PLAN: - verify there are no regressions to old/new Popover - ensure new popover supported the same props/api as old popover - verify new popover throws deprecation warnings only when old api/props are used - unit tests should pass Change-Id: Ibc2547118ba1c042621df2924a9e71457badb60d Reviewed-on: https://gerrit.instructure.com/c/instructure-ui/+/213080 Reviewed-by: Steve Jensen <sejensen@instructure.com> Product-Review: Steve Jensen <sejensen@instructure.com> Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com> QA-Review: Daniel Sasaki <dsasaki@instructure.com> Visual-Regression-Test: Ken Meleta <kmeleta@instructure.com>
1 parent 9fec28b commit d9437ae

File tree

11 files changed

+210
-74
lines changed

11 files changed

+210
-74
lines changed

packages/instui-config/codemod-configs/v8/propNames.config.json

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
21
{
32
"Avatar": {
43
"8.0.0": [
@@ -75,6 +74,21 @@
7574
{ "old": "trackPosition", "new": "shouldTrackPosition" }
7675
]
7776
},
77+
"Popover": {
78+
"8.0.0": [
79+
{ "old": "show", "new": "isShowingContent" },
80+
{ "old": "label", "new": "screenReaderLabel" },
81+
{ "old": "alignArrow", "new": "shouldAlignArrow" },
82+
{ "old": "trackPosition", "new": "shouldTrackPosition" },
83+
{ "old": "defaultShow", "new": "defaultIsShowingContent" },
84+
{ "old": "onShow", "new": "onPositioned" },
85+
{ "old": "onDismiss", "new": "onHideContent" },
86+
{ "old": "variant", "new": "color", "values": [
87+
{ "old": "default", "new": "primary" },
88+
{ "old": "inverse", "new": "primary-inverse" }
89+
]}
90+
]
91+
},
7892
"View": {
7993
"8.0.0": [
8094
{ "old": "borderColor", "new": "borderColor", "values": [

packages/ui-date-input/src/DateInput/index.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,6 @@ class DateInput extends Component {
433433
positionTarget={this._input}
434434
shouldReturnFocus={false}
435435
shouldFocusContentOnTriggerBlur
436-
__dangerouslyIgnoreExperimentalWarnings
437436
>
438437
{this.renderCalendar({ getListProps, getOptionProps })}
439438
</Popover>

packages/ui-overlays/src/Popover/README.md

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ id: DeprecatedPopover__README
55

66
**DEPRECATED:** Popover will be removed from `ui-overlays` in version 7.0.0. Use the [Popover from ui-popover](#Popover) instead.
77

8-
### Important Upgrade Notes
9-
Codemods are available to automatically update imports to the new package as well as any props that have changed. However, there are some breaking changes that will need to be addressed manually. These changes and other things to note are described below.
10-
- `Popover.Trigger` and `Popover.Content` are no longer in use. The trigger is now defined via the `renderTrigger` prop and the `children` of a `Popover` will be rendered as the content.
11-
- Some callbacks have changed. Instead of `onShow`, `onDismiss`, and `onToggle`, there is only `onRequestShowContent` and `onRequestHideContent`. When Popover is controlled, these callbacks will also be fired as prompts for updating the `isShowingContent` prop.
12-
<br/><br/>
13-
148
Popovers are actionable containers that are triggered by click. When opened, it remains connected with element that triggered it. Popovers are on the same hierarchy as the [Tray](#Tray) but contains less content.
159

1610
```js

packages/ui-overlays/src/Popover/__tests__/Popover.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ describe('<Popover />', async () => {
160160
it('should call onToggle', async () => {
161161
const onToggle = spy()
162162
await mount(
163-
<Popover on="click" show={false} onToggle={onToggle}>
163+
<Popover on="click" onToggle={onToggle}>
164164
<PopoverTrigger>
165165
<button>Click Me</button>
166166
</PopoverTrigger>

packages/ui-overlays/src/Popover/index.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import { LayoutPropTypes } from '@instructure/ui-layout'
2929
import { Popover as UIPopover } from '@instructure/ui-popover'
3030
import { bidirectional } from '@instructure/ui-i18n'
3131
import { Children, controllable, element } from '@instructure/ui-prop-types'
32-
import { ComponentIdentifier } from '@instructure/ui-react-utils'
32+
import { ComponentIdentifier, deprecated } from '@instructure/ui-react-utils'
3333
import { ThemeablePropTypes } from '@instructure/ui-themeable'
3434
import { testable } from '@instructure/ui-testable'
3535

@@ -49,6 +49,7 @@ category: components/deprecated
4949
id: DeprecatedPopover
5050
---
5151
**/
52+
@deprecated('7.0.0', null, 'Use @instructure/ui-popover instead')
5253
@testable()
5354
@bidirectional()
5455
class Popover extends Component {
@@ -346,8 +347,10 @@ class Popover extends Component {
346347
color={variant === 'inverse' ? 'primary-inverse' : 'primary'}
347348
shouldAlignArrow={alignArrow}
348349
shouldTrackPosition={trackPosition}
349-
onRequestShowContent={() => onToggle(true)}
350-
onRequestHideContent={(e, { documentClick }) => {
350+
onShowContent={() => {
351+
onToggle(true)
352+
}}
353+
onHideContent={(e, { documentClick }) => {
351354
onDismiss(e, documentClick)
352355
onToggle(false)
353356
}}

packages/ui-popover/README.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
---
22
category: packages
3-
experimental: true
43
---
54

65
## ui-popover

packages/ui-popover/src/Popover/README.md

Lines changed: 28 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,30 @@
11
---
22
describes: Popover
33
---
4+
5+
```js
6+
---
7+
guidelines: true
8+
---
9+
<Guidelines>
10+
<Figure title="Upgrade Notes for v8.0.0" recommendation="none">
11+
<Figure.Item>
12+
<code>Popover.Trigger</code> and <code>Popover.Content</code> are no longer in use. The trigger is now defined via the <code>renderTrigger</code> prop and the <code>children</code> of a Popover will be rendered as the content.
13+
</Figure.Item>
14+
<Figure.Item>
15+
The <code>onToggle</code> callback has been replaced by <code>onShowContent</code> and <code>onHideContent</code> When controlled, these will also be called as prompts for updating the <code>isShowingContent</code> prop.
16+
</Figure.Item>
17+
<Figure.Item>
18+
The <code>onShow</code> callback has been renamed to <code>onPositioned</code>.
19+
</Figure.Item>
20+
</Figure>
21+
</Guidelines>
22+
23+
```
24+
425
Popovers hide or show content as a result of user interaction, such as clicking, hovering, or focusing. When opened, the content remains connected to the element that triggered it. If you only need to display a small amount of text-only content, you might consider using a [Tooltip](#Tooltip). If you need to display a larger amount of content, a [Tray](#Tray) could be a better choice.
526

27+
628
#### Uncontrolled Popover
729
```js
830
---
@@ -18,8 +40,8 @@ example: true
1840
placement="top center"
1941
mountNode={() => document.getElementById('main')}
2042
onPositioned={() => console.log('positioned')}
21-
onRequestShowContent={() => console.log('showing')}
22-
onRequestHideContent={() => console.log('hidden')}
43+
onShowContent={() => console.log('showing')}
44+
onHideContent={() => console.log('hidden')}
2345
>
2446
<View padding="x-small" display="block" as="div" id="tip">
2547
Hello World
@@ -62,10 +84,10 @@ class Example extends React.Component {
6284
<Popover
6385
renderTrigger={<Button>Sign In</Button>}
6486
isShowingContent={this.state.isShowingContent}
65-
onRequestShowContent={(e) => {
87+
onShowContent={(e) => {
6688
this.setState({ isShowingContent: true })
6789
}}
68-
onRequestHideContent={(e, { documentClick }) => {
90+
onHideContent={(e, { documentClick }) => {
6991
this.setState({ isShowingContent: false })
7092
}}
7193
on="click"
@@ -145,10 +167,10 @@ class Example extends React.Component {
145167
<Popover
146168
renderTrigger={<Button margin="small">focus me</Button>}
147169
isShowingContent={this.state.isShowingContent}
148-
onRequestShowContent={(e) => {
170+
onShowContent={(e) => {
149171
this.setState({ isShowingContent: true })
150172
}}
151-
onRequestHideContent={(e) => {
173+
onHideContent={(e) => {
152174
this.setState({ isShowingContent: false })
153175
}}
154176
on={['focus', 'click']}

packages/ui-popover/src/Popover/__tests__/Popover.test.js

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ describe('<Popover />', async () => {
5858
testEventHandler('onBlur', 'focus', 'blur')
5959

6060
it('should hide content when clicked outside content by default', async () => {
61-
const onRequestHideContent = spy()
61+
const onHideContent = spy()
6262
await mount(
6363
<Popover
6464
on="click"
65-
onRequestHideContent={onRequestHideContent}
65+
onHideContent={onHideContent}
6666
renderTrigger={<button>Click Me</button>}
6767
>
6868
<h2>Foo Bar Baz</h2>
@@ -88,15 +88,15 @@ describe('<Popover />', async () => {
8888
content = await popover.findContent({ expectEmpty: true })
8989

9090
expect(content).to.not.exist()
91-
expect(onRequestHideContent.lastCall.args[1].documentClick).to.be.true()
91+
expect(onHideContent.lastCall.args[1].documentClick).to.be.true()
9292
})
9393

9494
it('should hide content when trigger is clicked', async () => {
95-
const onRequestHideContent = spy()
95+
const onHideContent = spy()
9696
await mount(
9797
<Popover
9898
on="click"
99-
onRequestHideContent={onRequestHideContent}
99+
onHideContent={onHideContent}
100100
shouldCloseOnDocumentClick={false}
101101
renderTrigger={<button>Click Me</button>}
102102
>
@@ -113,7 +113,7 @@ describe('<Popover />', async () => {
113113
const content = await popover.findContent({ expectEmpty: true })
114114

115115
expect(content).to.not.exist()
116-
expect(onRequestHideContent.lastCall.args[1].documentClick).to.be.false()
116+
expect(onHideContent.lastCall.args[1].documentClick).to.be.false()
117117
})
118118

119119
it('should show content if defaultIsShowingContent is true', async () => {
@@ -171,16 +171,16 @@ describe('<Popover />', async () => {
171171
expect(content.getTextContent()).to.equal('Foo Bar Baz')
172172
})
173173

174-
it('should call onRequestShowContent and onRequestHideContent', async () => {
175-
const onRequestShowContent = spy()
176-
const onRequestHideContent = spy()
174+
it('should call onShowContent and onHideContent', async () => {
175+
const onShowContent = spy()
176+
const onHideContent = spy()
177177
const subject = await mount(
178178
<Popover
179179
on="click"
180180
isShowingContent={false}
181181
shouldCloseOnDocumentClick={false}
182-
onRequestShowContent={onRequestShowContent}
183-
onRequestHideContent={onRequestHideContent}
182+
onShowContent={onShowContent}
183+
onHideContent={onHideContent}
184184
renderTrigger={<button>Click Me</button>}
185185
>
186186
<h2>Foo Bar Baz</h2>
@@ -190,12 +190,12 @@ describe('<Popover />', async () => {
190190
const trigger = await popover.findTrigger()
191191

192192
await trigger.click()
193-
expect(onRequestShowContent).to.have.been.calledOnce()
193+
expect(onShowContent).to.have.been.calledOnce()
194194

195195
await subject.setProps({ isShowingContent: true })
196196

197197
await trigger.click()
198-
expect(onRequestHideContent.lastCall.args[1].documentClick).to.be.false()
198+
expect(onHideContent.lastCall.args[1].documentClick).to.be.false()
199199
})
200200

201201
it('should not show content on click', async () => {
@@ -221,13 +221,13 @@ describe('<Popover />', async () => {
221221

222222
describe('when shouldFocusContentOnTriggerBlur=true and shouldContainFocus=false', async () => {
223223
it('should move focus into the content when the trigger is blurred', async () => {
224-
const onRequestHideContent = spy()
224+
const onHideContent = spy()
225225
await mount(
226226
<span>
227227
<button>focus me first</button>
228228
<Popover
229229
isShowingContent={true}
230-
onRequestHideContent={onRequestHideContent}
230+
onHideContent={onHideContent}
231231
renderTrigger={<button>focus me</button>}
232232
on={['hover', 'focus', 'click']}
233233
mountNode={() => document.getElementById('container')}
@@ -260,7 +260,7 @@ describe('<Popover />', async () => {
260260
})
261261

262262
await button.keyDown('tab')
263-
expect(onRequestHideContent).to.have.been.calledOnce()
263+
expect(onHideContent).to.have.been.calledOnce()
264264
})
265265
})
266266
})

0 commit comments

Comments
 (0)