kalkih / mini-media-player

Minimalistic media card for Home Assistant Lovelace UI
MIT License
1.51k stars 206 forks source link

2022.3.X Breaks dropdown #606

Closed bramkragten closed 2 years ago

bramkragten commented 2 years ago

mini-media-player uses paper-menu-button and paper-listbox with the assumption Home Assistant will load these elements.

https://github.com/kalkih/mini-media-player/blob/master/src/components/dropdown.js#L57

In 2022.3 these elements are no longer part of Home Assistant, so this will no longer work.

kalkih commented 2 years ago

Thanks for the heads up, much appreciated. Any suggestions on elements registered by HA to replace them with? or should I move in the direction of bundling my own components with the card?

bramkragten commented 2 years ago

we replaced them with mwc-select but to prevent issues like this in the future we do advise to bundle your own.

donaldguy commented 2 years ago

Having fallen way too far down a rabbit hole of impostersyndromedog.png[^1] to justify my mere desire to get my dropdown to close (see closed issue just above), I tried to update this repo to Lit 2 and bundle in the needed/existing[^2] polymer components (and then integrated ESM import in a vein attempt to de-dupe the imports in the short term):

https://github.com/donaldguy/mini-media-player/commit/d643b8310e1e5eda59b41ebdb60914f675736a29

It will npm run webpack "build" (npm run build gets tripped up on eslint w/ the .jss on the directive imports - without which webpack says it can't find the exports )

But (despite it being the same error that motivated my attempt to replatform onto import right quick in the first place), this still just gets me:

2022-02-17 23:24:28 ERROR (MainThread) [frontend.js.latest.202202140] https://[redacted]/hacsfiles/mini-media-player-card/mini-media-player-bundle.js?v=0a0ead97-a3cb-48e8-85a7-814eddb149d6:503:0 NotSupportedError: CustomElementRegistry.define: 'dom-module' has already been defined as a custom element

(despite the /hacsfile there, I am pretty sure this is in fact the new bundle Iscped in and changed the UUID get param of; I deleted the cached version through web tools for good measure)


So I give up. Someone who has used any of these technologies before can take a crack at it - I'll swap back to entities or maybe downgrade idk.

Hopefully that commit can save you some of the busy work though?

[^1]:

[^2]: I think the typeaheads on the mwc-select seem nice, but I was trying to limit blast radius

Mariusthvdb commented 2 years ago

I guess this is what I am experiencing now: https://github.com/home-assistant/frontend/issues/11738 this happens without even touching the selector, and in all browsers.

Schermafbeelding 2022-02-19 om 22 32 21
donaldguy commented 2 years ago

Reverting to my backup at core-2022.3.0.dev20220213 (ghcr.io/home-assistant/qemux86-64-homeassistant@sha256:0a4f7cc00c2664ab8edcab19afe87e2f94584b99453256831cc0bd356c7e0aaf; Frontend version: 20220203.0 - latest) did resolve the issue for me for the time being

So we can gather the change is somewhere in here: https://github.com/home-assistant/frontend/compare/20220203.0...20220214.0

(I mean, it seems like the above explanation could still track, but just saying)

donaldguy commented 2 years ago

And I guess this is our template for going forward? https://github.com/home-assistant/frontend/pull/11543/files

donaldguy commented 2 years ago

Well more relevantly https://github.com/home-assistant/frontend/pull/11591/files

kalkih commented 2 years ago

Hey guys, thanks for the ideas and the discussion.

So I feel like the right thing going forward is to move away from HA defined elements, as using them cause the card to break every now and then. (I think?) HA also defines some elements in runtime so user's might experience different behaviour depending on setup.

The drawback being the increased bundle size, but since at least the mwc element are built with Lit (which I already bundle with the card), the difference shouldn't be massive.

Here's a branch with mwc-menu bundled and with the elements defined under different names to avoid the duplicate elements issue.

It's untested in HA and themeing is probably not working correctly with HA defined variables. If anyone want to test it out and/or continue to tweak it, please do.

donaldguy commented 2 years ago

No dice on the branch as yet:

2022-02-22 14:12:09 ERROR (MainThread) [frontend.js.latest.202202200] /hacsfiles/mini-media-player/mini-media-player-bundle.js?hacstag=1485208381151:1:0 NotSupportedError: CustomElementRegistry.define: 'mmp-checkbox' has already been defined as a custom element 2022-02-22 14:12:15 ERROR (MainThread) [frontend.js.latest.202202200] /hacsfiles/mini-media-player/mini-media-player-bundle.js?hacstag=1485208381151:1:0 NotSupportedError: CustomElementRegistry.define: 'mwc-ripple' has already been defined as a custom element

Not immediately sure how hard/possible it is to shadow out those sub-elements too.


I do (still) suspect tho that it would be okay to both ship the mwc components (so you are sure you can depend on them) and also, in practice, preferentially import the HA-shipped versions (avoiding the double def), using new-fangled module/manifest dynamic import, but I admit I don't fully understand how that (would) work/s

(and the impression that it can work may be informed by looking in devtools at a less-productionized webback build on the core frontend that's potentially an artifact of pulling dev channel builds)

kalkih commented 2 years ago

I do (still) suspect tho that it would be okay to both ship the mwc components (so you are sure you can depend on them) and also, in practice

That's exactly what I did with mwc-menu and mwc-list-item in that branch, I extend the base classes of those components and define the elements with my custom names so they won't clash with the mwc elements defined by HA. Altough if these components have other mwc-components as dependecies I'm not sure if this is a viable approach anyway.

https://github.com/kalkih/mini-media-player/commit/ba4291a47834c5912edf4251e1955f4f6a76fd7d#diff-0d1e9e111ee4cf01c8b8f80f8a4e0e2fd735e4ae2e82abf2bf8c24f840791e5aR11-R24

The first error you posted seems strange though, mmp-checkbox should for sure not be defined more than once, make sure you're not loading mini-media-player multiple times, e.g. multiple resource references to mini-media-player in your setup.

bramkragten commented 2 years ago

We have good news, we found a way for custom cards to define the same elements HA uses! More info and examples on this shortly.

bramkragten commented 2 years ago

Here is an example how mwc elements can be used in custom cards in the next Home Assistant version:

https://github.com/custom-cards/boilerplate-card/pull/50/files

bramkragten commented 2 years ago

Or a simpler example here:

https://github.com/piitaya/lovelace-mushroom/issues/133#issuecomment-1047826814

We are shipping the scoped-custom-element-registry polyfill with the next HA release

kalkih commented 2 years ago

Thanks @bramkragten,

Might not be an issue, haven't read up on the Scoped Custom Element Registries spec fully yet. But will we not potentially still have issues when mwc-elements are imported in the base classes of the mwc-elements? Such as here https://github.com/material-components/material-web/blob/master/packages/list/mwc-check-list-item-base.ts#L11

regevbr commented 2 years ago

I managed to migrate the mini-climate card (after a lot of trail and error, and based on the boilerplate repo mentioned here) - https://github.com/artem-sedykh/mini-climate-card/pull/67 you can use it as a reference if that helps to fix this repo

hmmbob commented 2 years ago

Can confirm this card's dropdown is broken on 2022.3:

image

bramkragten commented 2 years ago

Thanks @bramkragten,

Might not be an issue, haven't read up on the Scoped Custom Element Registries spec fully yet. But will we not potentially still have issues when mwc-elements are imported in the base classes of the mwc-elements? Such as here https://github.com/material-components/material-web/blob/master/packages/list/mwc-check-list-item-base.ts#L11

That's why we also define those, and ignore those imports in the rollup config.

So it needs some "hacks", but it works...

kalkih commented 2 years ago

Until I've bundled the mwc-elements with the card this fix should do: #616

Beta release here: https://github.com/kalkih/mini-media-player/releases/tag/v1.16.0-beta

Let me know

hmmbob commented 2 years ago

Jep, beta fixes the issue for me

Screenshot_20220302-191046_Home Assistant Screenshot_20220302-191032_Home Assistant

vadimbz commented 2 years ago

I copied beta version to \www\community\mini-media-player\, but broken dropdown is still there. What am I missing? Thanks!

tomlut commented 2 years ago

Delete the .gz file

You can install beta versions with HACS. just tick the beta versions box.

vadimbz commented 2 years ago

Reinstalling from HACS worked, thanks!

niekniek89 commented 2 years ago

upgrading to beta version fixed the issue!

JimRnewell commented 2 years ago

upgrading to beta version has worked for me, thank you

pepsonEL commented 2 years ago

Update to 2022.3 for me some breaks card.... source from media player fix show on other entity.... screen

Ulrar commented 2 years ago

Confirming that the beta solves the issue after upgrading to 2022.3 this morning. thanks ! I would expect a lot of users to end up here in the coming days since it just got released

kalkih commented 2 years ago

Fix now also available in latest stable release.

tnagels commented 2 years ago

v1.16 which was just released does not solve the problem for me; drop-downs in the card config do not work.

Edit: typo in version

hmmbob commented 2 years ago

Try clearing your cache

tnagels commented 2 years ago

I did that. I'm seeing the same behaviour on Chrome, Edge & the HA app. HA has been restarted and the caches cleared.

MarcelS1988 commented 2 years ago

On chrome the problem is solved, but on IOS and Android tablet with fully kiosk still the same problem.

hmmbob commented 2 years ago

Highly likely a caching issue?

MarcelS1988 commented 2 years ago

On IOS you can not clear the caching? On Tablet I have done it but still it will not work. Or if you have any other ideas?

stronheim commented 2 years ago

Delete the .gz file

You can install beta versions with HACS. just tick the beta versions box.

Where do you find the .gz file to delete it?

I've updated the the beta and the problem is persisting for me.

hmmbob commented 2 years ago

You don't need the beta, just upgrade the card through HACS and clear caches

stronheim commented 2 years ago

You don't need the beta, just upgrade the card through HACS and clear caches

I've done the upgrade and cleared the caches on my phone and my pc and the problem is persisting. Can't quite work out why it's persisting

MarcelS1988 commented 2 years ago

Here the update works on the pc but not on android tablet and IPhone

Verstuurd vanaf mijn iPhone

Op 11 mrt. 2022 om 15:04 heeft stronheim @.***> het volgende geschreven:

I've done the upgrade and cleared the caches on my phone and my pc and the problem is persisting. Can't quite work out why it's persisting

alex-gillies commented 2 years ago

This is my configuration UI in version 1.16.1, HA 2022.4.7, Chrome 101.0.4951.41. I've cleared cache, restarted HA and host, hard refreshed, reinstalled HACS and Mini Media Player. Apologies if I'm missing something in troubleshooting, but I'm kind of at a loss at this point. I'm able to work around it with the code editor but would really like to get this fixed if possible.

Any advice would be greatly appreciated

ConfigUI

kalkih commented 2 years ago

@alex-gillies For some reason the minor releases of the card seems to unpublish themselves on github... I've republished the latest version (1.16.2), Check for this version in HACS, this version should include fixes for the issues you're seeing.

alex-gillies commented 2 years ago

@kalkih Fixed! Thanks for the quick turnaround!