From 8c0a78bbb7031fc210e09bd2e5e6103ad0725115 Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Sun, 24 Sep 2017 21:26:52 +0900 Subject: [PATCH] Refactor findbar with Aphrodite Closes #9233 - Apply BrowserButton to the close button - Apply Textbox to the input element - Add color styles to theme.js Auditors: Test Plan: 1. Run `npm run test -- --grep='findbar'` 2. Open facebook.com 3. Display find bar 4. Make sure the bar is displayed properly --- app/renderer/components/main/findbar.js | 195 +++++++++++++++++------ app/renderer/components/styles/global.js | 4 +- app/renderer/components/styles/theme.js | 29 ++++ js/entry.js | 1 - less/findbar.less | 102 ------------ less/variables.less | 2 - test/lib/selectors.js | 10 +- 7 files changed, 182 insertions(+), 161 deletions(-) delete mode 100644 less/findbar.less diff --git a/app/renderer/components/main/findbar.js b/app/renderer/components/main/findbar.js index 8a4c2edcd54..854748cfaa3 100644 --- a/app/renderer/components/main/findbar.js +++ b/app/renderer/components/main/findbar.js @@ -4,7 +4,7 @@ const React = require('react') const Immutable = require('immutable') -const {StyleSheet, css} = require('aphrodite') +const {StyleSheet, css} = require('aphrodite/no-important') // Components const ReduxComponent = require('../reduxComponent') @@ -29,6 +29,9 @@ const cx = require('../../../../js/lib/classSet') // Styles const globalStyles = require('../styles/global') +const commonStyles = require('../styles/commonStyles') +const {theme} = require('../styles/theme') +const {commonFormStyles} = require('../common/commonForm') class FindBar extends React.Component { constructor (props) { @@ -183,18 +186,22 @@ class FindBar extends React.Component { numberOfMatches: this.props.numberOfMatches } - return
+ data-l10n-id='findResults' + data-test-id='foundResults' + /> } else if (this.props.numberOfMatches === 0 && this.props.searchString) { const l10nArgs = { activeMatchOrdinal: this.props.activeMatchOrdinal, numberOfMatches: this.props.numberOfMatches } - return
+ data-l10n-id='findResultMatches' + data-test-id='foundResults' + /> } return null @@ -240,58 +247,146 @@ class FindBar extends React.Component { } } - return
-
-
- - { this.searchInput = node }} - type='text' - spellCheck='false' - onContextMenu={this.onContextMenu} - onFocus={this.onInputFocus} - onKeyDown={this.onKeyDown} - onInput={this.onInput} - /> - -
- {this.findTextMatch} - +
+ + { this.searchInput = node }} + type='text' + spellCheck='false' + onContextMenu={this.onContextMenu} + onFocus={this.onInputFocus} + onKeyDown={this.onKeyDown} + onInput={this.onInput} + data-test-id='findBarInput' /> -
- + {this.findTextMatch} + + + + + + + testId='findBarClearButton' + />
} } +const styles = StyleSheet.create({ + findBar: { + background: theme.findBar.backgroundColor, + borderBottom: `1px solid ${theme.findBar.border.bottom.color}`, + color: theme.findBar.color, + fontSize: '.7rem', + display: 'flex', + alignItems: 'center', + justifyContent: 'center', + padding: '.4rem .8rem', + animation: 'slideIn 25ms', + cursor: 'default', + WebkitAppRegion: 'no-drag' + }, + + findBar__string: { + display: 'flex', + alignItems: 'center', + position: 'relative' + }, + + findBar__string__icon: { + position: 'relative', + margin: 0, + padding: 0, + width: 0, + color: theme.findBar.string.icon.color + }, + + findBar__string__icon_search: { + left: '8px' + }, + + findBar__string__icon_clear: { + left: '-20px' + }, + + findBar__string__input: { + height: '25px', + width: '200px', + padding: '0 25px' + }, + + findBar__find: { + minWidth: '60px', + margin: '0 10px', + color: theme.findBar.find.color, + textAlign: 'center' + }, + + findBar__close: { + position: 'absolute', + right: '.8rem', + + ':hover': { + color: theme.findBar.close.onHover.color + } + } +}) + module.exports = ReduxComponent.connect(FindBar) diff --git a/app/renderer/components/styles/global.js b/app/renderer/components/styles/global.js index 65162337cef..1269a5286c9 100644 --- a/app/renderer/components/styles/global.js +++ b/app/renderer/components/styles/global.js @@ -73,7 +73,6 @@ const globalStyles = { switchBG_off_lrg: '#adadad', switchBG_dis: '#e8e8e8', switchNubColor: 'white', - findbarBackground: '#F7F7F7', veryLightGray: 'rgb(250, 250, 250)', lightGray: 'rgb(236, 236, 236)', gray: 'rgb(153, 153, 153)', @@ -252,6 +251,9 @@ const globalStyles = { question: 'fa fa-question-circle', refresh: 'fa fa-refresh', remove: 'fa fa-times', + resume: 'fa fa-play', + retry: 'fa fa-repeat', + search: 'fa fa-search', trash: 'fa fa-trash', unlock: 'fa fa-unlock', user: 'fa fa-user', diff --git a/app/renderer/components/styles/theme.js b/app/renderer/components/styles/theme.js index f9c6063fe0d..2e08db1405d 100644 --- a/app/renderer/components/styles/theme.js +++ b/app/renderer/components/styles/theme.js @@ -2,6 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this file, * You can obtain one at http://mozilla.org/MPL/2.0/. */ + const globalStyles = require('./global') + /** * Includes color options for theming * This should be used as a boilerplate for all @@ -63,5 +65,32 @@ } } } + }, + + findBar: { + backgroundColor: '#F7F7F7', + color: globalStyles.color.highlightBlue, + + border: { + bottom: { + color: globalStyles.color.lightGray + } + }, + + string: { + icon: { + color: globalStyles.color.gray + } + }, + + find: { + color: '#555' + }, + + close: { + onHover: { + color: 'inherit' + } + } } } diff --git a/js/entry.js b/js/entry.js index db56a538267..c945c053313 100644 --- a/js/entry.js +++ b/js/entry.js @@ -10,7 +10,6 @@ require('../less/navigationBar.less') require('../less/forms.less') require('../less/switchControls.less') require('../less/tabs.less') -require('../less/findbar.less') require('../less/downloadBar.less') require('../less/bookmarksToolbar.less') require('../less/notificationBar.less') diff --git a/less/findbar.less b/less/findbar.less deleted file mode 100644 index 84af992c566..00000000000 --- a/less/findbar.less +++ /dev/null @@ -1,102 +0,0 @@ -/* This Source Code Form is subject to the terms of the Mozilla Public - * License, v. 2.0. If a copy of the MPL was not distributed with this file, - * You can obtain one at http://mozilla.org/MPL/2.0/. */ - -@import "variables.less"; - -.findBar { - -webkit-app-region: no-drag; - background: @findbarBackground; - border-bottom: 1px solid @lightGray; - color: @highlightBlue; - display: flex; - font-size: 12px; - align-items: center; - justify-content: center; - padding: 5px 10px; - animation: slideIn 25ms; - cursor: default; - - * { - user-select: none; - } - - .findMatchText { - margin: 0 10px; - min-width: 60px; - text-align: center; - } - - label { - color: @mediumGray; - } - - .closeButton { - border: 1px solid transparent; - color: @gray; - cursor: default; - font-size: 22px; - opacity: 0.5; - transform: rotate(45deg); - - &:not([disabled]):hover { - opacity: 1; - } - - &.hideButton:not([disabled]):hover { - background: @chromeControlsWarningBackground; - } - } - - .searchContainer { - display: flex; - align-items: center; - margin: 0 auto; - } - - .searchStringContainer { - display: inline-block; - position: relative; - - span { - color: @gray; - padding: 0 5px; - } - - input { - height: 24px; - width: 200px; - margin-right: 5px; - padding: 0 25px; - border: none; - border-radius: @borderRadius; - } - - .foundResults { - color: #555; - font-size: 10px; - margin-right: 8px; - position: absolute; - right: 0; - top: 5px; - } - } - - .searchStringContainerIcon { - position: absolute; - line-height: 24px; - - &.fa-times { - right: 10px; - } - } - - .caseSensitivityContainer { - color: @gray; - margin-top: 6px; - - input { - margin-right: 6px; - } - } -} diff --git a/less/variables.less b/less/variables.less index 92b32854536..a6123b1e7f0 100644 --- a/less/variables.less +++ b/less/variables.less @@ -95,8 +95,6 @@ @navbarBraveButtonMarginLeft: 80px; @navbarLeftMarginDarwin: 76px; -@findbarBackground: #F7F7F7; - @carotRadius: 8px; @sideBarWidth: 190px; diff --git a/test/lib/selectors.js b/test/lib/selectors.js index 9b971c3d26e..789287d6dea 100644 --- a/test/lib/selectors.js +++ b/test/lib/selectors.js @@ -28,13 +28,13 @@ module.exports = { errorContent: '.errorContent', errorUrl: '.errorUrl', errorText: '.errorText', - findBar: '.findBar', - findBarInput: '.searchStringContainer input', - findBarMatches: '.foundResults', - findBarMatchCase: '#caseSensitivityCheckbox', + findBar: '[data-test-id="findBar"]', + findBarInput: '[data-test-id="findBarInput"]', + findBarMatches: '[data-test-id="foundResults"]', + findBarMatchCase: '[data-test-id="caseSensitivityCheckbox"]', findBarNextButton: '[data-test-id="findBarNextButton"]', findBarPrevButton: '[data-test-id="findBarPrevButton"]', - findBarClearButton: '.findClear', + findBarClearButton: '[data-test-id="findBarClearButton"]', braveMenu: '.braveMenu:not(.braveShieldsDisabled)', braveMenuDisabled: '.braveMenu.braveShieldsDisabled',