material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.15k stars 2.15k forks source link

[MDCSelect] 4.0.0 inconsistency - "focus-trap": "^5.0.0" #5223

Closed cintaccs closed 4 years ago

cintaccs commented 5 years ago

"@material/dialog": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/@material/dialog/-/dialog-4.0.0.tgz", "integrity": "sha512-ATfmcRKjPUuhpjKuCCtYrbuRfx1vZ9r0ZkMI2eLBxWbYbTbKOH+oM5Z24uVO38bX7SvOItPWsAQZYkoiyPUdPQ==", "requires": { "@material/animation": "^4.0.0", "@material/base": "^4.0.0", ..... "focus-trap": "^5.0.0", "tslib": "^1.9.3" }

and

"@material/drawer": { "version": "4.0.0", ... "focus-trap": "^5.0.0",

not sure whether it has any impact on anything - I do however have some unexplained (no errors - no exceptions) - weird random display of mdc-select ...

cintaccs commented 5 years ago

like this:

Screenshot 2019-11-04 at 03 51 00

the "Language" floating header and the outline should be a line higher. The Dateformat outline the same.

It is not consistently wrong. Next reload - it will be correct. But then not repeatable for a few times.

abhiomkar commented 5 years ago

Can you throw something in Glitch sandbox or codepen to see this demo? Note that MDC-select HTML structure is changed in 4.0.0

cintaccs commented 5 years ago

Thanks for the feedback! I am aware of the changed structure and have changed my code accordingly - it is rather big and complex and a SPA with all sort of dependencies so difficult to narrow down. However - when it fails - I saw that the DOM had the older select behaviour of moving the mdc-list content to the bottom of the body instead of the new (and much better! thanks!!) use of the child embedded HTML mdc-list. I guess that your new version of the code - would no longer "move" the DOM elements - therefore I was looking for reasons that could trigger the old behaviour. The old behaviour (grabbing the mdc-list and inserting at the end of body) is for sure not in my code. I was wondering if some fall-back or IE11 polyfill or whatever in your bundled material-components-web.min.js could be the reason? (that is why I looked in the package-lock file for inconsistencies).

So I guess the question is whether there by any chance would still be cases where the old "move HTML in the DOM" behaviour would be active? You know the specific code that did that operation and can maybe judge whether that code is still in material-components-web.min.js?

cintaccs commented 5 years ago
Screenshot 2019-11-04 at 13 20 00

I can't get the code formatter in Github to show easily readable - but here is the "template" I use to insert select html. As you can see it is delivered embedded under the parent select node.

Screenshot 2019-11-04 at 13 22 08

And the init of the selectors isn't doing that either.

bonniezhou commented 5 years ago

Hmm everything seems to look fine from your HTML structure. Which browser/OS is this on?

cintaccs commented 5 years ago

Chrome latest - I have seen the problem many times through Cypress also - (it runs Chromium I think). My guess is that something is failing in my code elsewhere on the page or some sort of race-condition - where the old "hoisted" menu code somehow kicks in. I am currently setting up my webpack to be able to run your source SCSS files - maybe I can try the component js instead of the bigger distribution of js (the one you ship under material-components-web) and debug it. I haven't seen the issue today - I had it many times yesterday. I also proved that the code was freshly generated (I tagged the code you see above with a dummy class) and the mdc-list was moved to the bottom (which I am almost 100% sure I do not do anywhere in my code)...

cintaccs commented 5 years ago

it happens both when I host and browse from Mac / Linux and Windows 10 btw. (I npm build my code on Debian and Win10). I browse using latest Chrome from Mac and Windows.

cintaccs commented 5 years ago
Screenshot 2019-11-06 at 03 08 06

You might actually have a point asking the OS's .... there are differences in the npm packages that Debian pulls and what Windows is pulling down on "npm i" command....

cintaccs commented 4 years ago

I haven't seen this behaviour since the first days after the implementation of 4.0 I guess it very well could be some mix of package-lock in between Linux and Windows versions of NPM.