Skip to content
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Attempt to fix Failed to execute 'removeChild' on 'Node' #9196

Merged
merged 4 commits into from
Aug 17, 2022
Merged

Attempt to fix Failed to execute 'removeChild' on 'Node' #9196

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a69919
Switch tooltips to use React Portals
t3chguy Aug 17, 2022
3c1d4e3
Remove redundant React key to simplify reconciliation
t3chguy Aug 17, 2022
f1fe5aa
Fix cleanup and it.each test
t3chguy Aug 17, 2022
73edcf8
Update snapshots due to style order difference
t3chguy Aug 17, 2022
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
8 changes: 4 additions & 4 deletions src/HtmlUtils.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -632,13 +632,13 @@ export function topicToHtml(
emojiBodyElements = formatEmojis(topic, false);
}

return isFormattedTopic ?
<span
key="body"
return isFormattedTopic
? <span
ref={ref}
dangerouslySetInnerHTML={{ __html: safeTopic }}
dir="auto"
/> : <span key="body" ref={ref} dir="auto">
/>
: <span ref={ref} dir="auto">
{ emojiBodyElements || topic }
</span>;
}
Expand Down
70 changes: 37 additions & 33 deletions src/components/views/elements/Tooltip.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@ export interface ITooltipProps {
maxParentWidth?: number;
}

export default class Tooltip extends React.Component<ITooltipProps> {
private tooltipContainer: HTMLElement;
type State = Partial<Pick<CSSProperties, "display" | "right" | "top" | "transform" | "left">>;

export default class Tooltip extends React.PureComponent<ITooltipProps, State> {
private static container: HTMLElement;
private parent: Element;

// XXX: This is because some components (Field) are unable to `import` the Tooltip class,
Expand All @@ -65,37 +67,47 @@ export default class Tooltip extends React.Component<ITooltipProps> {
alignment: Alignment.Natural,
};

// Create a wrapper for the tooltip outside the parent and attach it to the body element
constructor(props) {
super(props);

this.state = {};

// Create a wrapper for the tooltips and attach it to the body element
if (!Tooltip.container) {
Tooltip.container = document.createElement("div");
Tooltip.container.className = "mx_Tooltip_wrapper";
document.body.appendChild(Tooltip.container);
}
}

public componentDidMount() {
this.tooltipContainer = document.createElement("div");
this.tooltipContainer.className = "mx_Tooltip_wrapper";
document.body.appendChild(this.tooltipContainer);
window.addEventListener('scroll', this.renderTooltip, {
window.addEventListener('scroll', this.updatePosition, {
passive: true,
capture: true,
});

this.parent = ReactDOM.findDOMNode(this).parentNode as Element;

this.renderTooltip();
this.updatePosition();
}

public componentDidUpdate() {
this.renderTooltip();
this.updatePosition();
}

// Remove the wrapper element, as the tooltip has finished using it
public componentWillUnmount() {
ReactDOM.unmountComponentAtNode(this.tooltipContainer);
document.body.removeChild(this.tooltipContainer);
window.removeEventListener('scroll', this.renderTooltip, {
window.removeEventListener('scroll', this.updatePosition, {
capture: true,
});
}

// Add the parent's position to the tooltips, so it's correctly
// positioned, also taking into account any window zoom
private updatePosition(style: CSSProperties) {
private updatePosition = (): void => {
// When the tooltip is hidden, no need to thrash the DOM with `style` attribute updates (performance)
if (!this.props.visible) return;

const parentBox = this.parent.getBoundingClientRect();
const width = UIStore.instance.windowWidth;
const spacing = 6;
Expand All @@ -112,6 +124,7 @@ export default class Tooltip extends React.Component<ITooltipProps> {
parentBox.left - window.scrollX + (parentWidth / 2)
);

const style: State = {};
switch (this.props.alignment) {
case Alignment.Natural:
if (parentBox.right > width / 2) {
Expand Down Expand Up @@ -153,40 +166,31 @@ export default class Tooltip extends React.Component<ITooltipProps> {
break;
}

return style;
}

private renderTooltip = () => {
let style: CSSProperties = {};
// When the tooltip is hidden, no need to thrash the DOM with `style`
// attribute updates (performance)
if (this.props.visible) {
style = this.updatePosition({});
}
// Hide the entire container when not visible. This prevents flashing of the tooltip
// if it is not meant to be visible on first mount.
style.display = this.props.visible ? "block" : "none";
this.setState(style);
};

public render() {
const tooltipClasses = classNames("mx_Tooltip", this.props.tooltipClassName, {
"mx_Tooltip_visible": this.props.visible,
"mx_Tooltip_invisible": !this.props.visible,
});

const style = { ...this.state };
// Hide the entire container when not visible.
// This prevents flashing of the tooltip if it is not meant to be visible on first mount.
style.display = this.props.visible ? "block" : "none";

const tooltip = (
<div className={tooltipClasses} style={style}>
<div className="mx_Tooltip_chevron" />
{ this.props.label }
</div>
);

// Render the tooltip manually, as we wish it not to be rendered within the parent
ReactDOM.render<Element>(tooltip, this.tooltipContainer);
};

public render() {
// Render a placeholder
return (
<div className={this.props.className} />
<div className={this.props.className}>
{ ReactDOM.createPortal(tooltip, Tooltip.container) }
</div>
);
}
}
39 changes: 18 additions & 21 deletions test/components/views/elements/TooltipTarget-test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,14 @@ describe('<TooltipTarget />', () => {
'data-test-id': 'test',
};

afterEach(() => {
// clean up renderer tooltips
const wrapper = document.querySelector('.mx_Tooltip_wrapper');
while (wrapper?.firstChild) {
wrapper.removeChild(wrapper.lastChild);
}
});

const getComponent = (props = {}) => {
const wrapper = renderIntoDocument<HTMLSpanElement>(
// wrap in element so renderIntoDocument can render functional component
Expand All @@ -49,31 +57,20 @@ describe('<TooltipTarget />', () => {

const getVisibleTooltip = () => document.querySelector('.mx_Tooltip.mx_Tooltip_visible');

afterEach(() => {
// clean up visible tooltips
const tooltipWrapper = document.querySelector('.mx_Tooltip_wrapper');
if (tooltipWrapper) {
document.body.removeChild(tooltipWrapper);
}
});

it('renders container', () => {
const component = getComponent();
expect(component).toMatchSnapshot();
expect(getVisibleTooltip()).toBeFalsy();
});

for (const alignment in Alignment) {
if (isNaN(Number(alignment))) {
it(`displays ${alignment} aligned tooltip on mouseover`, () => {
const wrapper = getComponent({ alignment: Alignment[alignment] });
act(() => {
Simulate.mouseOver(wrapper);
});
expect(getVisibleTooltip()).toMatchSnapshot();
});
}
}
const alignmentKeys = Object.keys(Alignment).filter((o: any) => isNaN(o));
it.each(alignmentKeys)("displays %s aligned tooltip on mouseover", async (alignment) => {
const wrapper = getComponent({ alignment: Alignment[alignment] });
act(() => {
Simulate.mouseOver(wrapper);
});
expect(getVisibleTooltip()).toMatchSnapshot();
});

it('hides tooltip on mouseleave', () => {
const wrapper = getComponent();
Expand Down Expand Up @@ -101,8 +98,8 @@ describe('<TooltipTarget />', () => {
Simulate.focus(wrapper);
});
expect(getVisibleTooltip()).toBeTruthy();
await act(async () => {
await Simulate.blur(wrapper);
act(() => {
Simulate.blur(wrapper);
});
expect(getVisibleTooltip()).toBeFalsy();
});
Expand Down
14 changes: 7 additions & 7 deletions test/components/views/elements/__snapshots__/TooltipTarget-test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`<TooltipTarget /> displays Bottom aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="top: 6px; left: 0px; transform: translate(-50%); display: block;"
style="display: block; top: 6px; left: 0px; transform: translate(-50%);"
>
<div
class="mx_Tooltip_chevron"
Expand All @@ -15,7 +15,7 @@ exports[`<TooltipTarget /> displays Bottom aligned tooltip on mouseover 1`] = `
exports[`<TooltipTarget /> displays InnerBottom aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="top: -50px; left: 0px; transform: translate(-50%); display: block;"
style="display: block; top: -50px; left: 0px; transform: translate(-50%);"
>
<div
class="mx_Tooltip_chevron"
Expand All @@ -27,7 +27,7 @@ exports[`<TooltipTarget /> displays InnerBottom aligned tooltip on mouseover 1`]
exports[`<TooltipTarget /> displays Left aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="right: 1030px; top: 0px; transform: translateY(-50%); display: block;"
style="display: block; right: 1030px; top: 0px; transform: translateY(-50%);"
>
<div
class="mx_Tooltip_chevron"
Expand All @@ -39,7 +39,7 @@ exports[`<TooltipTarget /> displays Left aligned tooltip on mouseover 1`] = `
exports[`<TooltipTarget /> displays Natural aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="left: 6px; top: 0px; transform: translateY(-50%); display: block;"
style="display: block; left: 6px; top: 0px; transform: translateY(-50%);"
>
<div
class="mx_Tooltip_chevron"
Expand All @@ -51,7 +51,7 @@ exports[`<TooltipTarget /> displays Natural aligned tooltip on mouseover 1`] = `
exports[`<TooltipTarget /> displays Right aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="left: 6px; top: 0px; transform: translateY(-50%); display: block;"
style="display: block; left: 6px; top: 0px; transform: translateY(-50%);"
>
<div
class="mx_Tooltip_chevron"
Expand All @@ -63,7 +63,7 @@ exports[`<TooltipTarget /> displays Right aligned tooltip on mouseover 1`] = `
exports[`<TooltipTarget /> displays Top aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="top: -6px; left: 0px; transform: translate(-50%, -100%); display: block;"
style="display: block; top: -6px; left: 0px; transform: translate(-50%, -100%);"
>
<div
class="mx_Tooltip_chevron"
Expand All @@ -75,7 +75,7 @@ exports[`<TooltipTarget /> displays Top aligned tooltip on mouseover 1`] = `
exports[`<TooltipTarget /> displays TopRight aligned tooltip on mouseover 1`] = `
<div
class="mx_Tooltip test tooltipClassName mx_Tooltip_visible"
style="top: -6px; right: 1024px; transform: translateY(-100%); display: block;"
style="display: block; top: -6px; right: 1024px; transform: translateY(-100%);"
>
<div
class="mx_Tooltip_chevron"
Expand Down