-
-
Notifications
You must be signed in to change notification settings - Fork 827
Fix room list being laggy while scrolling π #7939
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -98,6 +98,8 @@ export default class Tooltip extends React.Component<ITooltipProps> { | |
}); | ||
} | ||
|
||
// Add the parent's position to the tooltips, so it's correctly | ||
// positioned, also taking into account any window zoom | ||
private updatePosition(style: CSSProperties) { | ||
const parentBox = this.parent.getBoundingClientRect(); | ||
let offset = 0; | ||
|
@@ -155,11 +157,12 @@ export default class Tooltip extends React.Component<ITooltipProps> { | |
} | ||
|
||
private renderTooltip = () => { | ||
// Add the parent's position to the tooltips, so it's correctly | ||
// positioned, also taking into account any window zoom | ||
// NOTE: The additional 6 pixels for the left position, is to take account of the | ||
// tooltips chevron | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the extra 6 pixels note because that is no longer relevant since the tooltip positioning was made to be relative to the element itself, see #7551 |
||
const style = this.updatePosition({}); | ||
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({}); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only update the |
||
// 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"; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,6 +44,19 @@ const TooltipTarget: React.FC<IProps> = ({ | |
const show = () => setIsVisible(true); | ||
MadLittleMods marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const hide = () => setIsVisible(false); | ||
|
||
// No need to fill up the DOM with hidden tooltip elements. Only add the | ||
// tooltip when we're hovering over the item (performance) | ||
const tooltip = isVisible && <Tooltip | ||
id={id} | ||
className={className} | ||
tooltipClassName={tooltipClassName} | ||
label={label} | ||
yOffset={yOffset} | ||
alignment={alignment} | ||
visible={isVisible} | ||
maxParentWidth={maxParentWidth} | ||
/>; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't add DOM elements to the page for hidden tooltips. This is the same pattern that we use in Previously, we just had a bunch of hidden |
||
|
||
return ( | ||
<div | ||
tabIndex={0} | ||
|
@@ -56,16 +69,7 @@ const TooltipTarget: React.FC<IProps> = ({ | |
{...rest} | ||
> | ||
{ children } | ||
<Tooltip | ||
id={id} | ||
className={className} | ||
tooltipClassName={tooltipClassName} | ||
label={label} | ||
yOffset={yOffset} | ||
alignment={alignment} | ||
visible={isVisible} | ||
maxParentWidth={maxParentWidth} | ||
/> | ||
{ tooltip } | ||
</div> | ||
); | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,8 +22,5 @@ exports[`<TooltipTarget /> renders container 1`] = ` | |
<span> | ||
child | ||
</span> | ||
<div | ||
class="test className" | ||
/> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because we selectively render |
||
</div> | ||
`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this comment up to describe the function instead of in the usage where the context can get lost when you're just focusing on the
updatePosition
code