Generalise media browse dialog from media player #7467
Conversation
|
Hi @rob-mccann, It seems you haven't yet signed a CLA. Please do so here. Once you do that we will be able to review and accept this pull request. Thanks! |
rob-mccann
force-pushed
the
media-browse-dialog
branch
from
f632ba4
to
77a5db3
Compare
| import { computeRTLDirection } from "../../common/util/compute_rtl"; | ||
| import { debounce } from "../../common/util/debounce"; | ||
| import type { MediaPlayerItem } from "../../data/media-player"; | ||
| import { fireEvent } from "../common/dom/fire_event"; |
There was a problem hiding this comment.
Why did you move all these files?
There was a problem hiding this comment.
I moved the dialog-related items to the dialog folder which left no need for the additional media-player directory in components so I moved ha-media-player-browse.ts up a level.
There was a problem hiding this comment.
But why? The media browser is related to the media player, so it makes sense to have it all together there?
There was a problem hiding this comment.
the idea is to allow the media browser to be used outside of the media player - for example, in a card editor to select media content. I don't feel too strongly about this though as it'll work for this usecase irrespective of location :)
| fireEvent(element, "show-dialog", { | ||
| dialogTag: "dialog-media-player-browse", | ||
| dialogImport: () => | ||
| import( | ||
| /* webpackChunkName: "dialog-media-player-browse" */ "./dialog-media-player-browse" | ||
| ), | ||
| dialogParams, | ||
| }); | ||
| fireEvent(element, "hass-media-browse", dialogParams); |
There was a problem hiding this comment.
The media browser dialog should be lazy-loaded.
There was a problem hiding this comment.
I've updated the event listener to lazy load rather than defer load.
|
Not sure I understand what you are trying to do here, I think all you want is already there. The only thing you can't do right now is import the dialog. But if it is imported (so after you triggered it from the normal UI), you should be able to use: fireEvent(element, "show-dialog", {
dialogTag: "dialog-media-player-browse",
dialogParams,
}); |
|
@bramkragten I'm trying to launch the browser from a different, custom card - there's no guarantee that it has previously been triggered. |
rob-mccann
force-pushed
the
media-browse-dialog
branch
from
77a5db3
to
912f518
Compare
|
The idea is to decouple the browser from the media player - to allow
browsing for other purposes (like component configuration).
I can certainly move it back if preferred - the location of the file isn’t
as important as the other changes here :)
…On Fri, 23 Oct 2020 at 22:11, Bram Kragten ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/components/ha-media-player-browse.ts
<#7467 (comment)>
:
> @@ -22,29 +22,29 @@ import {
import { classMap } from "lit-html/directives/class-map";
import { ifDefined } from "lit-html/directives/if-defined";
import { styleMap } from "lit-html/directives/style-map";
-import { fireEvent } from "../../common/dom/fire_event";
-import { computeRTLDirection } from "../../common/util/compute_rtl";
-import { debounce } from "../../common/util/debounce";
-import type { MediaPlayerItem } from "../../data/media-player";
+import { fireEvent } from "../common/dom/fire_event";
But why? The media browser is related to the media player, so it makes
sense to have it all together there?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7467 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADEYSB4B7TIZB7RVDP2BNDSMHWRNANCNFSM4S4M452Q>
.
|
|
I still don't understand what you want to do with the dialog, but this should not be a mixin and should just be handled by the normal dialog logic. |
|
I wish to create a new card in lovelace. Now - this card can be configured via YAML but I'd rather provide a UI that allows the user to select the media to show on the card via the media browser. Hope that makes sense? |
…-player-browse-dialog` event
rob-mccann
force-pushed
the
media-browse-dialog
branch
from
912f518
to
c366ff2
Compare
|
@bramkragten I've refactored the PR based on your feedback - do let me know if I've put all this in the right place - notably the mixin. |
|
This is not the way we handle dialogs, you used the deprecated old way. We also are not going to make our code much more complicated to support custom cards. I'm going to close this. |
|
This is really something we'd like to have. this way we can call the mediabrowser card in a popup for a specific media player. Please reconsider @bramkragten |
|
Hi, I really want to be able to call the media browser popup from a button, I don't use the default media player and I don't use the sidebar, is there any chance that you will reconsider this @bramkragten ? I have searched a lot for this and we are many who really need that feature 🙏 Thanks a lot in advance 😀 |
|
@rob-mccann Did you ever manage to achieve launching the media browser via a custom button/card ? - I'm trying to do the same and have been searching for days but no luck. |
Unfortunately, I didn't make any more progress after the MR was closed. Apologies! |
Allows other components and custom components to prompt for users to select an item from the library.
I'm new to HA and feeling my way through the codebase so please do take a good look and let me know if it makes sense!
Proposed change
Use case: I'd like to build a custom(?) card that launches a specific xbox game without having to find the content IDs.
The idea is to trigger this component from a new input type in the card configuration UI to fill the config.
Moving it to an event driven form means decouples it from the media player and allows it to be called from anywhere, including custom cards in the future.
Type of change
Additional information
Checklist