-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 option to disable controls on video and audio #1503
⚡ Add option to disable controls on video and audio #1503
Conversation
@michaldziuba03 is attempting to deploy a commit to the Typebot Team on Vercel. A member of the Team first needs to authorize it. |
Important Auto 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 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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
autoplay={ | ||
(props.onTransitionEnd ? false : true) || | ||
props.content?.hideControls | ||
} |
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.
I decided to autoplay the video as it would be impossible to play the video without controls.
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.
This check is here to avoid replaying all the media bubbles when we return to a chat session (after a refresh or else).
So this is need to stay as is otherwise, all the media without controls would play all at once on a page refresh if the chat state is saved
@baptisteArno ready for review |
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.
Thank you for that work 👌
I'd prefer if the option is an enabled instead of a disabler. So instead of having a Hide controls
option, I'd prefer having a Display controls
option that is true
by default.
You can add a default option in the constants
file (i.e.
import { AudioBubbleBlock } from './schema' |
autoplay={ | ||
(props.onTransitionEnd ? false : true) || | ||
props.content?.hideControls | ||
} |
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.
This check is here to avoid replaying all the media bubbles when we return to a chat session (after a refresh or else).
So this is need to stay as is otherwise, all the media without controls would play all at once on a page refresh if the chat state is saved
apps/builder/src/features/blocks/bubbles/audio/components/AudioBubbleForm.tsx
Outdated
Show resolved
Hide resolved
autoplay={props.onTransitionEnd ? false : true} | ||
autoplay={ | ||
props.onTransitionEnd | ||
? !( | ||
props.content?.areControlsDisplayed ?? | ||
defaultVideoBubbleContent.areControlsDisplayed | ||
) | ||
: 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.
I am not sure if this is correct
I took logic from AudioBubble.tsx
typebot.io/packages/embeds/js/src/features/blocks/bubbles/audio/components/AudioBubble.tsx
Lines 61 to 66 in e755f08
autoplay={ | |
props.onTransitionEnd | |
? props.content?.isAutoplayEnabled ?? | |
defaultAudioBubbleContent.isAutoplayEnabled | |
: false | |
} |
I guess disabling the controls on the audio element does not make sense as it hides the audio element altogether! |
Extends the implementation of #1503 as per the suggestions provided in the code review to resolve #1485 https://github.com/baptisteArno/typebot.io/assets/69730155/87481d64-57f5-4f7e-8a28-4a464f12cc31 --------- Co-authored-by: Baptiste Arnaud <[email protected]>
Thanks again for the work on this. I am closing this in favor of #1631 🙏 |
This PR adds option to remove controls for Videos (for now only URL type) and Audio bubbles.
typebot-pr.mp4
Closes #1485