Skip to content

Commit

Permalink
fix(ui): popover - useless element render
Browse files Browse the repository at this point in the history
  • Loading branch information
artyorsh authored Mar 18, 2019
1 parent b688ee8 commit 903f766
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 41 deletions.
17 changes: 8 additions & 9 deletions src/framework/ui/overflowMenu/overflowMenu.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,18 +51,17 @@ export class OverflowMenu extends React.Component<Props> {


private onSelect = (event: GestureResponderEvent, index: number): void => {
const { visible, onSelect } = this.props;
// FIXME: this is due to popover renders always with the "opacity": 0,
// so popover can listen events even it's "invisible"
if (visible && onSelect) {
onSelect(event, index);
if (this.props.onSelect) {
this.props.onSelect(event, index);
}
};

private getPopoverStyle = (style: StyleType): StyleType => ({
backgroundColor: style.popoverBackgroundColor,
borderRadius: style.borderRadius,
});
private getPopoverStyle = (style: StyleType): StyleType => {
return {
backgroundColor: style.popoverBackgroundColor,
borderRadius: style.borderRadius,
};
};

private getMenuItemStyle = (style: StyleType, index: number): StyleType => {
const borderRadius: number = style.itemBorderRadius;
Expand Down
4 changes: 0 additions & 4 deletions src/framework/ui/overflowMenu/overflowMenu.spec.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,7 @@ exports[`@overflow-menu: component checks * component renders properly 1`] = `
onLayout={[Function]}
style={
Object {
"opacity": 0,
"position": "absolute",
"zIndex": -1,
}
}
tag={1}
Expand Down Expand Up @@ -500,9 +498,7 @@ exports[`@overflow-menu: component checks * single menu-item 1`] = `
onLayout={[Function]}
style={
Object {
"opacity": 0,
"position": "absolute",
"zIndex": -1,
}
}
tag={1}
Expand Down
34 changes: 6 additions & 28 deletions src/framework/ui/popover/popover.component.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import React from 'react';
import {
Animated,
View,
FlexStyle,
ViewProps,
Expand Down Expand Up @@ -36,8 +35,6 @@ interface PopoverProps {
visible?: boolean;
}

type MeasuredNode = React.ReactElement<{ children: MeasuredElement[] }>;

interface State {
layout: MeasureResult | undefined;
}
Expand All @@ -48,7 +45,6 @@ const TAG_CHILD: number = 0;
const TAG_CONTENT: number = 1;
const PLACEMENT_DEFAULT: Placement = Placements.BOTTOM;

// FIXME: re-implement somehow "visibility" behavior
export class Popover extends React.Component<Props, State> {

static defaultProps: Partial<Props> = {
Expand All @@ -60,7 +56,7 @@ export class Popover extends React.Component<Props, State> {
layout: undefined,
};

private containerRef: React.RefObject<MeasuredNode> = React.createRef();
private popoverElement: MeasuredElement = undefined;
private modalIdentifier: string = '';

public shouldComponentUpdate(nextProps: Props, nextState: State, nextContext: any): boolean {
Expand All @@ -81,12 +77,7 @@ export class Popover extends React.Component<Props, State> {
top: popoverPosition.y,
};

const { current: container } = this.containerRef;

// Retrieve `content` from popover children and clone it with measured position
const { [TAG_CONTENT]: popoverView } = container.props.children;

const popover: React.ReactElement<ModalComponentCloseProps> = React.cloneElement(popoverView, {
const popover: React.ReactElement<ModalComponentCloseProps> = React.cloneElement(this.popoverElement, {
style: style,
onRequestClose: onRequestClose,
});
Expand Down Expand Up @@ -168,39 +159,26 @@ export class Popover extends React.Component<Props, State> {
);
};

private renderComponentElement = (...children: MeasuredElement[]): MeasuredNode => {
// Store `containerRef` for later usage.
// This is needed to retrieve `content` and position it in future
//
// Notes: No way (?) to store `View` ref, so we need to use `Animated.View`

return (
<Animated.View ref={this.containerRef}>
{children}
</Animated.View>
);
};

public render(): MeasuringNode | React.ReactNode {
const { themedStyle, content, children } = this.props;

const { child, popover } = this.getComponentStyle(themedStyle);

const measuringChild: MeasuringElement = this.renderChildElement(children, child);
const measuringPopover: MeasuringElement = this.renderPopoverElement(content, popover);

if (this.state.layout === undefined) {
return this.renderPlaceholderElement(measuringChild, measuringPopover);
}

return this.renderComponentElement(measuringChild, measuringPopover);
this.popoverElement = measuringPopover;

return measuringChild;
}
}

const styles = StyleSheet.create({
popover: {
position: 'absolute',
opacity: 0,
zIndex: -1,
},
placeholder: {
opacity: 0,
Expand Down

0 comments on commit 903f766

Please sign in to comment.