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

Fix screen resizer memory leak for mdl layout #6

Merged
merged 19 commits into from
Jul 23, 2024

Conversation

jakelauritsen
Copy link

No description provided.

src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
@jakelauritsen jakelauritsen requested a review from ribrdb July 17, 2024 21:12
src/layout/layout.js Outdated Show resolved Hide resolved
@jakelauritsen jakelauritsen requested a review from ribrdb July 17, 2024 22:58
Copy link

@ribrdb ribrdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has several lint, build, and test failures

src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
@jakelauritsen jakelauritsen self-assigned this Jul 18, 2024
@jakelauritsen jakelauritsen requested a review from ribrdb July 18, 2024 20:08
@jakelauritsen
Copy link
Author

For testing I ran the mocha index test (which was only so helpful because everything has errors), and I'm duping everything over to the concatenated material.js file to verify on a local branch.

How are you seeing lint errors, and/or is there a better way to test?

@ribrdb
Copy link

ribrdb commented Jul 18, 2024 via email

@jakelauritsen
Copy link
Author

I tried using the testing steps you provided, I keep getting errors, and this is the result of trying to run the tests on master:
image

@ribrdb
Copy link

ribrdb commented Jul 19, 2024

Strange, tests are passing for me on master. Maybe try 'npm install' again?
Or if you merge from master the github action should work now.

Here's what I'm getting on your branch


[17:25:42] Failed to load external module @babel/register
[17:25:42] Requiring external module babel-register
[17:25:43] Using gulpfile /workspaces/material-design-lite/gulpfile.babel.js
[17:25:43] Starting 'clean'...
[17:25:43] Finished 'clean' after 19 ms
[17:25:43] Starting 'default'...
[17:25:43] Starting 'styles'...
[17:25:44] Starting 'styles-grid'...
[17:25:45] styles-grid all files 18.82 kB
[17:25:45] Finished 'styles-grid' after 880 ms
[17:25:47] styles all files 827.55 kB
[17:25:47] Finished 'styles' after 3.81 s
[17:25:47] Starting 'lint'...

src/layout/layout.js
  line 217  col 4  Unnecessary semicolon.

  ⚠  1 warning

jsDoc: Unexpected data in tag const at /workspaces/material-design-lite/src/layout/layout.js :
    18 |  'use strict';
    19 |
    20 |  /** @const @private */
---------------^
    21 |  const MEDIA_QUERY = window.matchMedia('(max-width: 1024px)');
    22 |  MEDIA_QUERY.onchange = screenSizeHandler;


1 code style error found.

events.js:170
      throw er; // Unhandled 'error' event
      ^
Error: JSHint failed for: src/layout/layout.js

@jakelauritsen jakelauritsen force-pushed the jakelauritsen/fix-mdl-layout-screen-resize-mem-leak branch from 1d2d166 to fde3ac6 Compare July 19, 2024 17:39
src/layout/layout.js Outdated Show resolved Hide resolved
@jakelauritsen
Copy link
Author

jakelauritsen commented Jul 19, 2024

Also let in the screenSizeHandler is not ES5 🙃

@jakelauritsen jakelauritsen requested a review from ribrdb July 19, 2024 20:00
@jakelauritsen jakelauritsen force-pushed the jakelauritsen/fix-mdl-layout-screen-resize-mem-leak branch from 7f5f53f to 03133a7 Compare July 22, 2024 16:28
@jakelauritsen jakelauritsen force-pushed the jakelauritsen/fix-mdl-layout-screen-resize-mem-leak branch from 03133a7 to c63e700 Compare July 22, 2024 17:48
src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
src/layout/layout.js Outdated Show resolved Hide resolved
@jakelauritsen jakelauritsen force-pushed the jakelauritsen/fix-mdl-layout-screen-resize-mem-leak branch from 30409eb to cbb40d5 Compare July 22, 2024 20:13
src/layout/layout.js Outdated Show resolved Hide resolved
@jakelauritsen jakelauritsen requested a review from ribrdb July 22, 2024 20:40
@rivernate rivernate merged commit 589db69 into mdl-1.x Jul 23, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants