Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

fix(Popup): avoid double rendering #1153

Merged
merged 15 commits into from
May 9, 2019
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### BREAKING CHANGES
- Rename `context` prop to `mountNode` in `PortalInner` @layershifter ([#1288](https://github.com/stardust-ui/react/pull/1288))
- Updated Teams' theme color palette values, removed color related site variables @mnajdova ([#1069](https://github.com/stardust-ui/react/pull/1069))
- Remove `defaultTarget` prop in `Popup` component @layershifter ([#1153](https://github.com/stardust-ui/react/pull/1153))

### Features
- Add default child a11y behavior to `Menu` related behaviors @silviuavram ([#1282](https://github.com/stardust-ui/react/pull/1282))
Expand All @@ -29,6 +30,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
- Add `mountNode` and `mountDocument` props to allow proper multi-window rendering ([#1288](https://github.com/stardust-ui/react/pull/1288))
- Added default and brand color schemes in Teams' theme @mnajdova ([#1069](https://github.com/stardust-ui/react/pull/1069))

### Fixes
- Fix double rendering of `Popup` component @layershifter ([#1153](https://github.com/stardust-ui/react/pull/1153))

<!--------------------------------[ v0.29.1 ]------------------------------- -->
## [v0.29.1](https://github.com/stardust-ui/react/tree/v0.29.1) (2019-05-01)
[Compare changes](https://github.com/stardust-ui/react/compare/v0.29.0...v0.29.1)
Expand Down
40 changes: 16 additions & 24 deletions packages/react/src/components/Popup/Popup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
import { ComponentEventHandler, ReactProps, ShorthandValue } from '../../types'

import { getPopupPlacement, applyRtlToOffset, Alignment, Position } from './positioningHelper'
import createPopperReferenceProxy from './createPopperReferenceProxy'

import PopupContent from './PopupContent'

Expand Down Expand Up @@ -130,9 +131,6 @@ export interface PopupProps
*/
target?: HTMLElement

/** Initial value for 'target'. */
defaultTarget?: HTMLElement

/** Element to be rendered in-place where the popup is defined. */
trigger?: JSX.Element

Expand Down Expand Up @@ -169,7 +167,6 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
}),
align: PropTypes.oneOf(ALIGNMENTS),
defaultOpen: PropTypes.bool,
defaultTarget: PropTypes.any,
inline: PropTypes.bool,
mountDocument: PropTypes.object,
mountNode: customPropTypes.domNode,
Expand Down Expand Up @@ -199,9 +196,9 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
mouseLeaveDelay: 500,
}

static autoControlledProps = ['open', 'target']
static autoControlledProps = ['open']

triggerDomElement = null
triggerRef = React.createRef<HTMLElement>() as React.MutableRefObject<HTMLElement>
// focusable element which has triggered Popup, can be either triggerDomElement or the element inside it
triggerFocusableDomElement = null
popupDomElement = null
Expand Down Expand Up @@ -267,7 +264,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps

const isOutsidePopupElement = this.popupDomElement && !isInsideNested
const isOutsideTriggerElement =
this.triggerDomElement && !doesNodeContainClick(this.triggerDomElement, e)
this.triggerRef.current && !doesNodeContainClick(this.triggerRef.current, e)

return isOutsidePopupElement && isOutsideTriggerElement
}
Expand Down Expand Up @@ -396,12 +393,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
const triggerProps = this.getTriggerProps(triggerElement)
return (
triggerElement && (
<Ref
innerRef={domNode => {
this.trySetState({ target: domNode })
this.triggerDomElement = domNode
}}
>
<Ref innerRef={this.triggerRef}>
{React.cloneElement(triggerElement, {
...accessibility.attributes.trigger,
...triggerProps,
Expand All @@ -418,7 +410,7 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
accessibility: AccessibilityBehavior,
): JSX.Element {
const { align, position, offset } = this.props
const { target } = this.state
const { target } = this.props

const placement = getPopupPlacement({ align, position, rtl })

Expand All @@ -430,15 +422,15 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
}),
}

const referenceElement = createPopperReferenceProxy(target || this.triggerRef)

return (
target && (
<Popper
placement={placement}
referenceElement={target}
children={this.renderPopperChildren.bind(this, popupPositionClasses, rtl, accessibility)}
modifiers={popperModifiers}
/>
)
<Popper
placement={placement}
referenceElement={referenceElement}
children={this.renderPopperChildren.bind(this, popupPositionClasses, rtl, accessibility)}
modifiers={popperModifiers}
/>
)
}

Expand Down Expand Up @@ -570,8 +562,8 @@ export default class Popup extends AutoControlledComponent<ReactProps<PopupProps
const { mountDocument } = this.props
const activeElement = mountDocument.activeElement

this.triggerFocusableDomElement = this.triggerDomElement.contains(activeElement)
this.triggerFocusableDomElement = this.triggerRef.current.contains(activeElement)
? activeElement
: this.triggerDomElement
: this.triggerRef.current
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { isRefObject, toRefObject } from '@stardust-ui/react-component-ref'
import * as _ from 'lodash'
import * as React from 'react'
import * as PopperJS from 'popper.js'

class ReferenceProxy implements PopperJS.ReferenceObject {
ref: React.RefObject<HTMLElement>

constructor(refObject) {
this.ref = refObject
}

getBoundingClientRect() {
return _.invoke(this.ref.current, 'getBoundingClientRect', {})
}

get clientWidth() {
return this.getBoundingClientRect().width
}

get clientHeight() {
return this.getBoundingClientRect().height
}
}

/**
* Popper.js does not support ref objects from `createRef()` as referenceElement. If we will pass
* directly `ref`, `ref.current` will be `null` at the render process.
*
* @see https://popper.js.org/popper-documentation.html#referenceObject
* @see https://github.com/FezVrasta/react-popper/blob/v1.3.3/src/Popper.js#L166
*/
const createPopperReferenceProxy = (reference: HTMLElement | React.RefObject<HTMLElement>) => {
const referenceRef = isRefObject(reference) ? reference : toRefObject(reference)

return new ReferenceProxy(referenceRef)
}

export default createPopperReferenceProxy