PrivateCoffee/element-web
4
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 4

Fix room list being laggy while scrolling 🐌 (#7939)

Browse source 
Fix https://github.com/vector-im/element-web/issues/21262

Optimizations:

 1. Don't update the `style` (positioning) of hidden tooltips
 1. Don't add DOM elements to the page for hidden tooltips

> ## Performance problems broken down
>
>
> ### Hidden tooltips rendering on `scroll`
>
> You can see that the Tooltip render is attached to the `scroll` event  at [`src/components/views/elements/Tooltip.tsx#L78-L81`](31f0a37ca2/src/components/views/elements/Tooltip.tsx (L78-L81))
>
> The rendering calls [`src/components/views/elements/Tooltip.tsx#L101` -> `updatePosition`](36adba101c/src/components/views/elements/Tooltip.tsx (L101)) which ends up as an expensive "Recalculate Style" because it uses [`Element.getBoundingClientRect()`](https://developer.mozilla.org/en-US/docs/Web/API/Element/getBoundingClientRect). This happens many many times within a single `scroll` event. Probably once for each tooltip within the room list **even though no tooltips are event visible as I scroll**. I can see that we're just updating the `style` attribute for a bunch of `.mx_Tooltip_invisible` elements at the end of the document.
>
> Each one of the purple spans below the `scroll` span ends up as a call to `updatePosition`. And a `scroll` event takes 35ms to 60ms to complete which is way over the 16.6ms 60 FPS budget (on a powerful desktop PC), granted these times are with the performance profiling running. This is without the Passbolt extension explained below.
>
> And the room list contains about 141 rooms (`document.querySelectorAll('.mx_RoomTile').length`):
>
> ![](https://user-images.githubusercontent.com/558581/156273551-e744d3d6-93c6-4b07-bb12-6aad361f96a2.png)
>
>
>
> ### Passbolt Chrome browser extension exacerbates the problem
>
> In order to login to Passbolt, it requires a browser extension which defaults to mucking up all pages:
>
> <img src="https://user-images.githubusercontent.com/558581/156275644-bc26b1f5-5d99-4eae-b74b-c2028f2f1baf.png" width="300">
>
>
> The extension source seems to be available: https://github.com/passbolt/passbolt_browser_extension
>
> The Passbolt Chrome extension has a `MutationObserver` listening to all attribute and element changes to the whole `<body>` of the `document` so it can `findAndSetAuthenticationFields` (find form elements and autofill).
>
>
> [`passbolt/passbolt_styleguide` -> `src/react-web-integration/lib/InForm/InFormManager.js#L143`](1c5eddc910/src/react-web-integration/lib/InForm/InFormManager.js (L143))
> ```js
> this.mutationObserver.observe(document.body, { attributes: true, childList: true, subtree: true });
> ```
>
> This causes a bunch of `Forced reflow` because the Tooltip `updatePosition` is mutating the element `style` attribute and Passbolt `MutationObserver` callbacks are querying the whole DOM looking for form elements all in the same frame.
>
> Under the `scroll` event, all of the little spans are the `MutationObserver` -> `findAndSetAuthenticationFields`. With the Passbolt extension, scrolling is verrrrry crunchy and bad.
>
> ![](https://user-images.githubusercontent.com/558581/156144998-8cf7686f-3c7b-42f8-8d81-ff780bae0ab5.png)
>
>
> #### Workaround
>
> Instead of running Passbolt on all sites, we can enable the extension to only run on the domain for Passbolt instance itself. I'm guessing the Passbolt extension also does autofill stuff on sites but I always login manually to the Passbolt instance so this solution works for me �
>
> **Extensions** -> **Passbolt** -> **Details** -> Change **Site access** to `On specific sites` -> Enter in your Passbolt instance `https://passbolt.example.com/`
>
> ![](https://user-images.githubusercontent.com/558581/156275630-a53ef6a1-c058-4ac9-aa08-ae50b90e72c9.png)
>
> *-- https://github.com/vector-im/element-web/issues/21262*
This commit is contained in:
Eric Eastwood 2022-03-02 13:20:01 -06:00 committed by GitHub
parent f882466329
commit 3572b36648
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 25 additions and 19 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

13
src/components/views/elements/Tooltip.tsx
View file

@ -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) { private updatePosition(style: CSSProperties) {
const parentBox = this.parent.getBoundingClientRect(); const parentBox = this.parent.getBoundingClientRect();
let offset = 0; let offset = 0;
@ -155,11 +157,12 @@ export default class Tooltip extends React.Component<ITooltipProps> {
} }
private renderTooltip = () => { private renderTooltip = () => {
// Add the parent's position to the tooltips, so it's correctly let style: CSSProperties = {};
// positioned, also taking into account any window zoom // When the tooltip is hidden, no need to thrash the DOM with `style`
// NOTE: The additional 6 pixels for the left position, is to take account of the // attribute updates (performance)
// tooltips chevron if (this.props.visible) {
const style = this.updatePosition({}); style = this.updatePosition({});
}
// Hide the entire container when not visible. This prevents flashing of the tooltip // Hide the entire container when not visible. This prevents flashing of the tooltip
// if it is not meant to be visible on first mount. // if it is not meant to be visible on first mount.
style.display = this.props.visible ? "block" : "none"; style.display = this.props.visible ? "block" : "none";

24
src/components/views/elements/TooltipTarget.tsx
View file

@ -44,6 +44,19 @@ const TooltipTarget: React.FC<IProps> = ({
const show = () => setIsVisible(true); const show = () => setIsVisible(true);
const hide = () => setIsVisible(false); 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}
/>;
return ( return (
<div <div
tabIndex={0} tabIndex={0}
@ -56,16 +69,7 @@ const TooltipTarget: React.FC<IProps> = ({
{...rest} {...rest}
> >
{ children } { children }
<Tooltip { tooltip }
id={id}
className={className}
tooltipClassName={tooltipClassName}
label={label}
yOffset={yOffset}
alignment={alignment}
visible={isVisible}
maxParentWidth={maxParentWidth}
/>
</div> </div>
); );
}; };

2
test/components/views/elements/TooltipTarget-test.tsx
View file

@ -39,7 +39,9 @@ describe('<TooltipTarget />', () => {
afterEach(() => { afterEach(() => {
// clean up visible tooltips // clean up visible tooltips
const tooltipWrapper = document.querySelector('.mx_Tooltip_wrapper'); const tooltipWrapper = document.querySelector('.mx_Tooltip_wrapper');
if (tooltipWrapper) {
document.body.removeChild(tooltipWrapper); document.body.removeChild(tooltipWrapper);
}
}); });
it('renders container', () => { it('renders container', () => {

3
test/components/views/elements/__snapshots__/TooltipTarget-test.tsx.snap
View file

@ -22,8 +22,5 @@ exports[`<TooltipTarget /> renders container 1`] = `
<span> <span>
child child
</span> </span>
<div
class="test className"
/>
</div> </div>
`; `;