Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deregisterBodyClickListener not working - possible fix #1920

Closed
cintaccs opened this issue Oct 28, 2020 · 7 comments
Closed

deregisterBodyClickListener not working - possible fix #1920

cintaccs opened this issue Oct 28, 2020 · 7 comments
Assignees
Labels
Type: Bug Something isn't working

Comments

@cintaccs
Copy link

Screenshot 2020-10-28 at 09 48 59

activating - deactivating the menu's keeps registrering the click event listener on MDCMenuSurface.

Possible fix:
menu-surface component
component.js

MDCMenuSurface.prototype.initialSyncWithDOM
        this.deregisterBodyClickListener = function () {
//BEFORE: document.body.removeEventListener('click', _this.handleBodyClick);
//AFTER: add the , { capture: true } to the removeEventListener 
//to match the document.body.addEventListener('click', _this.handleBodyClick, { capture: true });
            document.body.removeEventListener('click', _this.handleBodyClick, { capture: true });
        };

FYI - I will post this suggestion on the material-components/material-components-web too

@dfreedm
Copy link
Collaborator

dfreedm commented Oct 29, 2020

@cintaccs
Copy link
Author

thanks for looking into this - my current "hack" is to do this (in lit-element updated):

this.temp_menu = new MDCMenu(div);
let origHandler = this.temp_menu.menuSurface_.handleBodyClick;
this.temp_menu.menuSurface_.deregisterBodyClickListener = () => {    
    document.body.removeEventListener('click', origHandler, { capture: true });
};

it's ugly - but seems to work.

@cintaccs
Copy link
Author

@dfreedm - just out of curiosity - may I ask why these types of memory leak issues are considered "Severity: Low"? As I see it there are a couple of them in the "framework" and when using any of the components (menu and tab bar seems rather widespread) in a SPA the events are adding up and up and up.... when building a solid SPA where users might work for hours - it seems to slow down and refiring etc....?

@zandaqo
Copy link

zandaqo commented Nov 24, 2020

I see it's been fixed in MDC, when is the next release with this fix expected? It's not just a memory leak, by the way, it messes up the logic when you subscribe to the 'closed' event of the menu surface--each undelegated handler emits an event resulting in an avalanche of events increasing the more times you open the menu.

@asyncLiz
Copy link
Collaborator

@zandaqo You can use the @canary version of MWC to get the latest version, which will have the MDC fix included, until the next release.

@zandaqo
Copy link

zandaqo commented Dec 24, 2020

The version 0.20.0 fixes this for me, but only when MDCMenuSurface#quick is set to true for some reason.

@asyncLiz
Copy link
Collaborator

asyncLiz commented Aug 2, 2023

Obsolete with M3

@asyncLiz asyncLiz closed this as not planned Won't fix, can't repro, duplicate, stale Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants