-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add stream preview element to generic camera #21463
base: dev
Are you sure you want to change the base?
Add stream preview element to generic camera #21463
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent updates improve URL handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FlowPreviewGeneric
participant FlowPreviewGenericCamera
participant HaHLSPlayer
User->>FlowPreviewGeneric: Request camera preview
FlowPreviewGeneric->>FlowPreviewGenericCamera: Render camera preview
FlowPreviewGenericCamera->>FlowPreviewGenericCamera: Check if _preview is available
alt _preview exists
FlowPreviewGenericCamera->>FlowPreviewGenericCamera: Extract stillUrl and streamUrl
alt stillUrl exists
FlowPreviewGenericCamera->>User: Display still image
else streamUrl exists
FlowPreviewGenericCamera->>HaHLSPlayer: Initialize video player
HaHLSPlayer->>User: Display video stream
end
else
FlowPreviewGenericCamera->>User: Display error alert
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
src/components/ha-hls-player.ts (1)
153-154
: Enhance the comment for clarity.Clarify the comment to explicitly mention the use of a try-catch block for error handling.
- // In case we arrive here with a relative URL, we need to provide a valid - // base/absolute URL to avoid the URL() constructor throwing an error. + // Use a try-catch block to handle cases where this.url is a relative URL. + // Provide a valid base/absolute URL to avoid the URL() constructor throwing an error.
src/dialogs/config-flow/previews/flow-preview-generic_camera.ts
Outdated
Show resolved
Hide resolved
bc26267
to
753c8b0
Compare
@balloob is this in line with what you meant in your comment on #21246? |
autoplay | ||
playsinline | ||
.hass=${this.hass} | ||
.controls=${false} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.controls=${false} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @bramkragten!
I assume this is because the default is false?
(sorry I am relatively new to frontend things)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the default is false, but also because it is a boolean, you can set it to true by just adding the attribute like you did for playsinline
for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
@bramkragten
Any ideas on how to deal with this? |
7329103
to
81d9510
Compare
The client side log messages are still present, but it seems to work ok |
I have tried rebasing this but it is no longer working correctly. Will take a little longer to figure out. Setting to draft for now. |
81d9510
to
5ad17d2
Compare
Now working again, after url was re-added to |
Proposed change
As part of a PR to enable a more user-friendly experience when configuring video stream sources, a preview element is added for previewing the still image and stream from a generic camera during configuration.
This is done by building on the
FlowPreviewGeneric
element, which had to be slightly modified to allow it to be subclassed.Also included is an svg spinner displayed by the video element while the stream is loading, from https://github.com/n3r4zzurr0/svg-spinners
Type of change
Additional information
Checklist
If user exposed functionality or configuration variables are added/changed:
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements