From c958c64ba8c825890bf28d3342935c8a99fdd63c Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Fri, 22 Jun 2018 15:12:23 -0700 Subject: [PATCH 1/2] ref(toolbar): remove use-new-toolbox class Very likely I broke something subtle and I'm prepared to fix it. --- css/_atlaskit_overrides.scss | 12 +++++ css/_chat.scss | 52 +++++-------------- css/_filmstrip.scss | 21 +++----- css/_side_toolbar_container.scss | 46 +++++++--------- css/_toolbars.scss | 11 ---- css/_vertical_filmstrip_overrides.scss | 23 ++++---- css/main.scss | 1 + css/modals/invite/_info.scss | 5 +- modules/UI/UI.js | 6 --- .../filmstrip/components/web/Filmstrip.js | 4 +- 10 files changed, 62 insertions(+), 119 deletions(-) create mode 100644 css/_atlaskit_overrides.scss diff --git a/css/_atlaskit_overrides.scss b/css/_atlaskit_overrides.scss new file mode 100644 index 000000000..cc2ca6dc5 --- /dev/null +++ b/css/_atlaskit_overrides.scss @@ -0,0 +1,12 @@ +/** + * Move Atlaskit Flag up a little bit so it does not cover the toolbar with the + * first notification. + */ +.cxGWJB{ + bottom: calc(#{$newToolbarSizeWithPadding}) !important; +} +.gXSEsl:nth-child(n+2) { + transform: translateX(0) translateY(100%) translateY(16px) !important; + -ms-transform: translateX(0) translateY(100%) translateY(16px) !important; + -webkit-transform: translateX(0) translateY(100%) translateY(16px) !important; +} diff --git a/css/_chat.scss b/css/_chat.scss index 9c0b3a59b..5d313bdc2 100644 --- a/css/_chat.scss +++ b/css/_chat.scss @@ -6,7 +6,7 @@ #chatconversation { visibility: hidden; position: relative; - top: 5px; + top: 15px; padding: 5px; text-align: left; line-height: 20px; @@ -60,37 +60,7 @@ } .localuser { - color: #087dba; -} -.use-new-toolbox { - .chatmessage { - color: white; - } - - .localuser { - color: #4C9AFF; - } - - .remoteuser { - color: #B8C7E0; - } - - #usermsg { - color: white; - } - - .chatmessage, - #smileysarea, - #smileysContainer, - #usermsg { - background-color: $newToolbarBackgroundColor; - } - - .smileyContainer:hover { - background-color: $newToolbarButtonToggleColor; - border-radius: 5px; - cursor: pointer; - } + color: #4C9AFF } .errorMessage { @@ -98,10 +68,11 @@ } .remoteuser { - color: white; + color: #B8C7E0; } #usermsg { + background-color: $newToolbarBackgroundColor; visibility:hidden; position: absolute; bottom: 0px; @@ -112,8 +83,7 @@ max-height:150px; min-height:35px; border: 0px none; - background: #3a3a3a; - color: #a7a7a7; + color: white; box-shadow: none; border-radius:0; font-size: 10pt; @@ -183,7 +153,7 @@ } .chatmessage { - background: #3a3a3a; + background-color: $newToolbarBackgroundColor;; width: 93%; margin-left: 9px; margin-right: auto; @@ -191,7 +161,7 @@ border-top-left-radius: 0px; margin-top: 3px; left: 5px; - color: #a7a7a7; + color: white; overflow: hidden; padding-bottom: 3px; } @@ -227,7 +197,7 @@ max-height:150px; min-height:35px; border: 0px none; - background: #3a3a3a; + background-color: $newToolbarBackgroundColor; overflow: hidden; visibility: hidden; } @@ -239,7 +209,7 @@ #smileysContainer { display: none; position: absolute; - background: #3a3a3a; + background-color: $newToolbarBackgroundColor; border-bottom: 1px solid; border-top: 1px solid; width: 100%; @@ -257,7 +227,9 @@ } .smileyContainer:hover { - background: #3e3e3e; + background-color: $newToolbarButtonToggleColor; + border-radius: 5px; + cursor: pointer; } #usermsg::-webkit-input-placeholder { diff --git a/css/_filmstrip.scss b/css/_filmstrip.scss index 5ea0ee36b..5db61d410 100644 --- a/css/_filmstrip.scss +++ b/css/_filmstrip.scss @@ -5,28 +5,19 @@ justify-content: flex-start; } -.use-new-toolbox { - .filmstrip.reduce-height { - bottom: $newToolbarSizeWithPadding; - } - - .filmstrip { - transition: bottom .3s; - } - - .filmstrip__videos.hidden { - bottom: calc(-196px - #{$newToolbarSizeWithPadding}); - } -} - .filmstrip { position: absolute; bottom: 0; right: 0; padding: 10px 5px; @extend %align-right; + transition: bottom .3s; z-index: $filmstripVideosZ; + &.reduce-height { + bottom: $newToolbarSizeWithPadding; + } + &__toolbar { @include flex(); flex-direction: column-reverse; @@ -83,7 +74,7 @@ } &.hidden { - bottom: -196px; + bottom: calc(-196px - #{$newToolbarSizeWithPadding}); } .remote-videos-container { diff --git a/css/_side_toolbar_container.scss b/css/_side_toolbar_container.scss index a0a5b8d34..710eab05a 100644 --- a/css/_side_toolbar_container.scss +++ b/css/_side_toolbar_container.scss @@ -1,41 +1,14 @@ /** * Toolbar side panel main container element. */ -.use-new-toolbox #sideToolbarContainer { +#sideToolbarContainer { background-color: $newToolbarBackgroundColor; - /** * Make the sidebar flush with the top of the toolbar. Take the size of * the toolbar and subtract from 100%. */ height: calc(100% - #{$newToolbarSizeWithPadding}); left: 0; - - .side-toolbar-close { - background: gray; - border: 3px solid rgba(255, 255, 255, 0.1); - border-radius: 100%; - color: white; - cursor:pointer; - height: 10px; - line-height: 10px; - padding: 4px; - position: absolute; - right: 5px; - text-align: center; - top: 5px; - width: 10px; - z-index: 1; - } - - #chatconversation { - top: 15px; - } -} -#sideToolbarContainer { - background-color: $sideToolbarContainerBg; - height: 100%; - left: $defaultToolbarSize; max-width: $sidebarWidth; overflow: hidden; position: absolute; @@ -112,4 +85,21 @@ margin-top: 0 !important; } } + + .side-toolbar-close { + background: gray; + border: 3px solid rgba(255, 255, 255, 0.1); + border-radius: 100%; + color: white; + cursor:pointer; + height: 10px; + line-height: 10px; + padding: 4px; + position: absolute; + right: 5px; + text-align: center; + top: 5px; + width: 10px; + z-index: 1; + } } diff --git a/css/_toolbars.scss b/css/_toolbars.scss index a8042edb7..a58700a4c 100644 --- a/css/_toolbars.scss +++ b/css/_toolbars.scss @@ -19,17 +19,6 @@ vertical-align: middle; } -.use-new-toolbox { - .cxGWJB{ - bottom: calc(#{$newToolbarSizeWithPadding}); - } - .gXSEsl:nth-child(n+2) { - transform: translateX(0) translateY(100%) translateY(16px); - -ms-transform: translateX(0) translateY(100%) translateY(16px); - -webkit-transform: translateX(0) translateY(100%) translateY(16px); - } -} - /** * TODO: when the old filmstrip has been removed, remove the "new-" prefix. */ diff --git a/css/_vertical_filmstrip_overrides.scss b/css/_vertical_filmstrip_overrides.scss index 291d56b68..e213b505a 100644 --- a/css/_vertical_filmstrip_overrides.scss +++ b/css/_vertical_filmstrip_overrides.scss @@ -19,20 +19,6 @@ text-align: left; } - &.use-new-toolbox { - /** - * Adjust the height of the filmstrip as the toolbar is displayed. - */ - .filmstrip { - top: 0; - transition: height .3s ease-in; - - &.reduce-height { - height: calc(100% - #{$newToolbarSizeWithPadding}); - } - } - } - .filmstrip { align-items: flex-end; box-sizing: border-box; @@ -46,8 +32,17 @@ * any parent is also fixed. */ position: fixed; + top: 0; + transition: height .3s ease-in; z-index: $filmstripVideosZ; + /** + * Adjust the height of the filmstrip as the toolbar is displayed. + */ + &.reduce-height { + height: calc(100% - #{$newToolbarSizeWithPadding}); + } + /** * Hide videos by making them slight to the right. */ diff --git a/css/main.scss b/css/main.scss index d96890336..3e38d4c47 100644 --- a/css/main.scss +++ b/css/main.scss @@ -31,6 +31,7 @@ /* Modules BEGIN */ @import 'aui_reset'; +@import 'atlaskit_overrides'; @import 'base'; @import 'utils'; @import 'overlay/overlay'; diff --git a/css/modals/invite/_info.scss b/css/modals/invite/_info.scss index 5fa66f917..191d3fb26 100644 --- a/css/modals/invite/_info.scss +++ b/css/modals/invite/_info.scss @@ -1,10 +1,7 @@ -.use-new-toolbox { - font-size: 14px; -} - .info-dialog { cursor: default; display: flex; + font-size: 14px; .info-dialog-action-link { display: inline-block; diff --git a/modules/UI/UI.js b/modules/UI/UI.js index dcb48c3f1..554496f53 100644 --- a/modules/UI/UI.js +++ b/modules/UI/UI.js @@ -319,12 +319,6 @@ UI.start = function() { // Initialize side panels SidePanels.init(eventEmitter); - - // TODO: remove this class once the old toolbar has been removed. This - // class is set so that any CSS changes needed to adjust elements - // outside of the new toolbar can be scoped to just the app with the new - // toolbar enabled. - $('body').addClass('use-new-toolbox'); } interfaceConfig.VERTICAL_FILMSTRIP diff --git a/react/features/filmstrip/components/web/Filmstrip.js b/react/features/filmstrip/components/web/Filmstrip.js index ec87006e4..96cf18494 100644 --- a/react/features/filmstrip/components/web/Filmstrip.js +++ b/react/features/filmstrip/components/web/Filmstrip.js @@ -108,7 +108,9 @@ class Filmstrip extends Component<*> { // will get updated without replacing the DOM. If the known DOM gets // modified, then the views will get blown away. const reduceHeight - = _toolboxVisible && interfaceConfig.TOOLBAR_BUTTONS.length; + = !_filmstripOnly + && _toolboxVisible + && interfaceConfig.TOOLBAR_BUTTONS.length; const classNames = `filmstrip ${ _remoteVideosVisible ? '' : 'hide-videos'} ${ From 880d3525db1643ceea0dd36fc5f41bc809c9235d Mon Sep 17 00:00:00 2001 From: Leonard Kim Date: Mon, 25 Jun 2018 08:33:08 -0700 Subject: [PATCH 2/2] squash: pass class name into filmstrip --- .../filmstrip/components/web/Filmstrip.js | 104 ++++++++---------- 1 file changed, 44 insertions(+), 60 deletions(-) diff --git a/react/features/filmstrip/components/web/Filmstrip.js b/react/features/filmstrip/components/web/Filmstrip.js index 96cf18494..a0967768b 100644 --- a/react/features/filmstrip/components/web/Filmstrip.js +++ b/react/features/filmstrip/components/web/Filmstrip.js @@ -1,7 +1,6 @@ /* @flow */ import _ from 'lodash'; -import PropTypes from 'prop-types'; import React, { Component } from 'react'; import { connect } from 'react-redux'; @@ -13,13 +12,40 @@ import Toolbar from './Toolbar'; declare var interfaceConfig: Object; +/** + * The type of the React {@code Component} props of {@link Filmstrip}. + */ +type Props = { + + /** + * Additional CSS class names top add to the root. + */ + _className: string, + + /** + * Whether the UI/UX is filmstrip-only. + */ + _filmstripOnly: boolean, + + /** + * Whether or not remote videos are currently being hovered over. Hover + * handling is currently being handled detected outside of react. + */ + _hovered: boolean, + + /** + * The redux {@code dispatch} function. + */ + dispatch: Dispatch<*> +}; + /** * Implements a React {@link Component} which represents the filmstrip on * Web/React. * * @extends Component */ -class Filmstrip extends Component<*> { +class Filmstrip extends Component { _isHovered: boolean; _notifyOfHoveredStateUpdate: Function; @@ -28,47 +54,13 @@ class Filmstrip extends Component<*> { _onMouseOver: Function; - /** - * {@code Filmstrip} component's property types. - * - * @static - */ - static propTypes = { - /** - * Whether the UI/UX is filmstrip-only. - */ - _filmstripOnly: PropTypes.bool, - - /** - * Whether or not remote videos are currently being hovered over. - */ - _hovered: PropTypes.bool, - - /** - * Whether or not the remote videos should be visible. Will toggle - * a class for hiding the videos. - */ - _remoteVideosVisible: PropTypes.bool, - - /** - * Whether or not the toolbox is visible. The height of the vertical - * filmstrip needs to adjust to accommodate the horizontal toolbox. - */ - _toolboxVisible: PropTypes.bool, - - /** - * The redux {@code dispatch} function. - */ - dispatch: PropTypes.func - }; - /** * Initializes a new {@code Filmstrip} instance. * * @param {Object} props - The read-only properties with which the new * instance is to be initialized. */ - constructor(props) { + constructor(props: Props) { super(props); // Debounce the method for dispatching the new filmstrip handled state @@ -95,30 +87,16 @@ class Filmstrip extends Component<*> { * @returns {ReactElement} */ render() { - const { - _filmstripOnly, - _remoteVideosVisible, - _toolboxVisible - } = this.props; - // Note: Appending of {@code RemoteVideo} views is handled through // VideoLayout. The views do not get blown away on render() because // ReactDOMComponent is only aware of the given JSX and not new appended // DOM. As such, when updateDOMProperties gets called, only attributes // will get updated without replacing the DOM. If the known DOM gets // modified, then the views will get blown away. - const reduceHeight - = !_filmstripOnly - && _toolboxVisible - && interfaceConfig.TOOLBAR_BUTTONS.length; - const classNames - = `filmstrip ${ - _remoteVideosVisible ? '' : 'hide-videos'} ${ - reduceHeight ? 'reduce-height' : ''}`; return ( -
- { _filmstripOnly && } +
+ { this.props._filmstripOnly && }
@@ -193,20 +171,26 @@ class Filmstrip extends Component<*> { * @param {Object} state - The Redux state. * @private * @returns {{ - * _filmstripOnly: boolean, + * _className: string, * _hovered: boolean, - * _remoteVideosVisible: boolean, - * _toolboxVisible: boolean + * _filmstripOnly: boolean * }} */ function _mapStateToProps(state) { const { hovered } = state['features/filmstrip']; + const isFilmstripOnly = Boolean(interfaceConfig.filmStripOnly); + const reduceHeight = !isFilmstripOnly + && state['features/toolbox'].visible + && interfaceConfig.TOOLBAR_BUTTONS.length; + const remoteVideosVisible = shouldRemoteVideosBeVisible(state); + + const className = `${remoteVideosVisible ? '' : 'hide-videos'} ${ + reduceHeight ? 'reduce-height' : ''}`; return { - _filmstripOnly: Boolean(interfaceConfig.filmStripOnly), - _hovered: hovered, - _remoteVideosVisible: shouldRemoteVideosBeVisible(state), - _toolboxVisible: state['features/toolbox'].visible + _className: className, + _filmstripOnly: isFilmstripOnly, + _hovered: hovered }; }