-
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?
Add button to open live chat popup in browser #8278
Conversation
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.
| <button | ||
| class="popoutChatButton" | ||
| :title="$t('Video.Popout Live Chat')" | ||
| @click="popoutChat" | ||
| > | ||
| <FontAwesomeIcon | ||
| class="popoutChatIcon" | ||
| :icon="['fas', 'fa-arrow-up-right-from-square']" | ||
| /> | ||
| </button> |
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 use a link instead of a button, as it is a link and using a button bypasses the external link handling setting.
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.
Nesting seems to be resolved but this isnt yet
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'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 <a> html link?
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'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
<a>html link?
Head branch was pushed to by a user without write access
Pull Request Type
Related issue
closes #1721
Description
Adds a button that opens a new browser window with the current video live chat
Screenshots
Testing
Go to a video that is listed as "Live"
Click on the "Popout chat" button
See that the current video live chat is opened in a new browser window
Additional context
h4label was replaced withspanbecause of margin was stretching button hover animationthe margin is now applied to the container, replicating the same size as before but the height is ~10px less now