-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add button to open live chat popup in browser #8278
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
base: development
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,15 +42,31 @@ | |
| v-else | ||
| class="relative" | ||
| > | ||
| <h4> | ||
| {{ t("Video.Live Chat") }} | ||
| <div | ||
| class="titleContainer" | ||
| > | ||
| <span | ||
| v-if="!hideVideoViews && watchingCount !== null" | ||
| class="watchingCount" | ||
| class="title" | ||
| > | ||
| {{ t('Global.Counts.Watching Count', { count: formattedWatchingCount }, watchingCount) }} | ||
| {{ t("Video.Live Chat") }} | ||
| <span | ||
| v-if="!hideVideoViews && watchingCount !== null" | ||
| class="watchingCount" | ||
| > | ||
| {{ t('Global.Counts.Watching Count', { count: formattedWatchingCount }, watchingCount) }} | ||
| </span> | ||
| </span> | ||
| </h4> | ||
| <button | ||
| class="popoutChatButton" | ||
| :title="$t('Video.Popout Live Chat')" | ||
| @click="popoutChat" | ||
| > | ||
| <FontAwesomeIcon | ||
| class="popoutChatIcon" | ||
| :icon="['fas', 'fa-arrow-up-right-from-square']" | ||
| /> | ||
| </button> | ||
|
Comment on lines
+59
to
+68
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please use a link instead of a button, as it is a link and using a button bypasses the external link handling setting.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nesting seems to be resolved but this isnt yet
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've been too busy to address this, but hopefully in a week or so can get to it. Just to confirm, it should be a regular
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| </div> | ||
| <div | ||
| v-if="superChatComments.length > 0" | ||
| class="superChatComments" | ||
|
|
@@ -571,6 +587,12 @@ function scrollToBottom() { | |
| stayAtBottom = true | ||
| showScrollToBottom.value = false | ||
| } | ||
|
|
||
| function popoutChat() { | ||
| const url = `https://www.youtube.com/live_chat?is_popout=1&v=${props.videoId}` | ||
| window.open(url, '_blank', 'noreferrer') | ||
| } | ||
|
|
||
| </script> | ||
|
|
||
| <style scoped src="./WatchVideoLiveChat.css" /> | ||
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.
Please remove the nesting, as it doesn't play well with Vue's scoped styles causing styles to "leak" into other components. In all the plain CSS files we've done it properly without nesting, the only reason that some SCSS files have nesting is because it is technical debt that we haven't cleaned up yet.
TODO for myself: Check if there is a stylelint rule we can use to enforce no-nesting with the linters.