From 8cc51c35bfa1ccb426b1a1ea528a51f39e39db9c Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Wed, 9 Aug 2017 18:54:40 +0900 Subject: [PATCH 1/3] Normalize the tables on about:preferences Addresses #10263 Addresses #10788 Closes #10356 - Polish action icons - Polish verified icon placement - Remove alignRight from the sites column (it should be aligned to the left as default) - Align the include switches to the center - Align the action buttons to the center Test Plan: 1. Visit https://www.youtube.com/user/latenight 2. Open about:preferences#payments 3. Make sure the channel title is aligned to the left --- .../components/common/sortableTable.js | 27 +++++-- .../preferences/payment/ledgerTable.js | 76 +++++++++++++------ .../preferences/payment/pinnedInput.js | 22 +++--- app/renderer/components/styles/global.js | 12 +++ js/about/preferences.js | 46 +++++++---- less/about/preferences.less | 13 +--- 6 files changed, 127 insertions(+), 69 deletions(-) diff --git a/app/renderer/components/common/sortableTable.js b/app/renderer/components/common/sortableTable.js index 704fce37842..461b18329c7 100644 --- a/app/renderer/components/common/sortableTable.js +++ b/app/renderer/components/common/sortableTable.js @@ -429,7 +429,7 @@ class SortableTable extends React.Component { data-test-id={this.stateOwner.state.selection.includes(this.getGlobalIndex(currentIndex)) ? 'selected' : null} data-row-index={`${currentIndex}`} className={cx({ - [css(styles.table__tbody__tr, this.props.largeRow && styles.table__tbody__tr_largeRow)]: true, + [css(styles.table__tbody__tr, this.props.smallRow && styles.table__tbody__tr_smallRow, this.props.largeRow && styles.table__tbody__tr_largeRow)]: true, [classes.join(' ')]: classes })}>{entry} : null @@ -482,7 +482,7 @@ class SortableTable extends React.Component { const headerClasses = { 'sort-header': true, 'sort-default': this.sortingDisabled || heading === this.props.defaultHeading, - [css(styles.table__th)]: true + [css(styles.table__th, this.props.smallRow && styles.table__th_smallRow)]: true } const isString = typeof heading === 'string' const sortMethod = this.sortingDisabled ? 'none' : (dataType === 'number' ? 'number' : undefined) @@ -532,12 +532,13 @@ const styles = StyleSheet.create({ }, table__th: { + boxSizing: 'border-box', background: `linear-gradient(to bottom, ${globalStyles.color.lightGray}, ${globalStyles.color.veryLightGray})`, borderTop: `1px solid ${globalStyles.color.gray}`, borderLeft: `1px solid ${globalStyles.color.braveOrange}`, textAlign: 'left', fontWeight: 300, - padding: '1ch', + padding: globalStyles.sortableTable.cell.normal.padding, whiteSpace: 'nowrap', // Up/down arrow @@ -554,6 +555,10 @@ const styles = StyleSheet.create({ } }, + table__th_smallRow: { + padding: globalStyles.sortableTable.cell.small.padding + }, + table__th__inner: { display: 'inline-block', userSelect: 'none' @@ -565,7 +570,12 @@ const styles = StyleSheet.create({ }, table__tbody__tr: { - height: '1rem' + boxSizing: 'border-box', + height: '1.75rem' + }, + + table__tbody__tr_smallRow: { + height: '1.5rem' }, // Add 'largeRow' to SortableTable to increase the height of tr. @@ -574,13 +584,14 @@ const styles = StyleSheet.create({ }, table__tbody__tr__td: { - padding: '.5ch 1ch' + boxSizing: 'border-box', + padding: `calc(${globalStyles.sortableTable.cell.normal.padding} / 2.25) ${globalStyles.sortableTable.cell.normal.padding}` }, - // Add 'smallRow' to SortableTable to decrease padding-top and padding-bottom of td. + // Add 'smallRow' to SortableTable to decrease padding values. table__tbody__tr_smallRow__td: { - paddingTop: '.3ch', - paddingBottom: '.3ch' + padding: `calc(${globalStyles.sortableTable.cell.small.padding} / 2.25) ${globalStyles.sortableTable.cell.small.padding}`, + height: '1.5rem' } }) diff --git a/app/renderer/components/preferences/payment/ledgerTable.js b/app/renderer/components/preferences/payment/ledgerTable.js index 5e9dee6eaed..b5f3dcb2b9e 100644 --- a/app/renderer/components/preferences/payment/ledgerTable.js +++ b/app/renderer/components/preferences/payment/ledgerTable.js @@ -3,7 +3,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const React = require('react') -const {StyleSheet, css} = require('aphrodite') +const {StyleSheet, css} = require('aphrodite/no-important') // components const ImmutableComponent = require('../../immutableComponent') @@ -115,12 +115,12 @@ class LedgerTable extends ImmutableComponent { get columnClassNames () { return [ css(styles.alignRight, styles.verifiedTd), // verified - css(styles.alignRight), // sites - css(styles.alignLeft), // include + css(styles.column_sites), // sites + css(styles.alignCenter), // include css(styles.alignRight), // views css(styles.alignRight), // time spent - css(styles.alignRight, styles.percTd), // percentage - css(styles.alignLeft) // actions + css(styles.alignRight, styles.column_percentage), // percentage + css(styles.alignCenter) // actions ] } @@ -203,7 +203,7 @@ class LedgerTable extends ImmutableComponent { value: duration }, { - html: + html: { pinned ? - + - - , + , value: '' } ] @@ -326,9 +333,9 @@ class LedgerTable extends ImmutableComponent { const verifiedBadge = (icon) => ({ height: '20px', width: '20px', - marginRight: '-10px', display: 'block', - background: `url(${icon}) center no-repeat` + background: `url(${icon}) center no-repeat`, + margin: 'auto' }) const gridStyles = StyleSheet.create({ @@ -367,11 +374,18 @@ const styles = StyleSheet.create({ }, verifiedTd: { - padding: '0 0 0 15px' + // Set the same padding with the padding-top and padding-bottom + paddingRight: `calc(${globalStyles.sortableTable.cell.small.padding} / 2) !important`, + paddingLeft: `calc(${globalStyles.sortableTable.cell.small.padding} / 2) !important` + }, + + column_sites: { + // TODO: Refactor sortableTable.less to remove !important + width: '100% !important' }, - percTd: { - width: '45px' + column_percentage: { + minWidth: '3ch' }, hideTd: { @@ -425,28 +439,44 @@ const styles = StyleSheet.create({ textAlign: 'left' }, + alignCenter: { + display: 'flex', + justifyContent: 'center' + }, + paymentsDisabled: { opacity: 0.6 }, - mainIcon: { + percentageValue: { + // To cancel the global color setting + color: '#656565' + }, + + actionIcons: { + display: 'flex', + alignItems: 'center', + justifyContent: 'space-around', + width: '50px', + margin: 'auto' + }, + + actionIcons__icon: { backgroundColor: '#c4c5c5', - width: '15px', - height: '16px', + width: '1rem', + height: '1rem', display: 'inline-block', - marginRight: '10px', - marginTop: '6px', ':hover': { backgroundColor: globalStyles.color.buttonColor } }, - pinIcon: { + actionIcons__icon_pin: { '-webkit-mask-image': `url(${pinIcon})` }, - pinnedIcon: { + actionIcons__icon_pin_isPinned: { backgroundColor: globalStyles.color.braveOrange, ':hover': { @@ -454,7 +484,7 @@ const styles = StyleSheet.create({ } }, - removeIcon: { + actionIcons__icon_remove: { '-webkit-mask-image': `url(${removeIcon})` }, diff --git a/app/renderer/components/preferences/payment/pinnedInput.js b/app/renderer/components/preferences/payment/pinnedInput.js index 9bae65dd35f..f0b8e71b59c 100644 --- a/app/renderer/components/preferences/payment/pinnedInput.js +++ b/app/renderer/components/preferences/payment/pinnedInput.js @@ -3,7 +3,7 @@ * You can obtain one at http://mozilla.org/MPL/2.0/. */ const React = require('react') -const {StyleSheet, css} = require('aphrodite') +const {StyleSheet, css} = require('aphrodite/no-important') // components const ImmutableComponent = require('../../immutableComponent') @@ -43,26 +43,28 @@ class PinnedInput extends ImmutableComponent { defaultValue={this.props.defaultValue} onBlur={this.pinPercentage.bind(this, this.props.patern)} onKeyPress={this.keyPress.bind(this)} - className={css(styles.percInput)} + className={css(styles.pinnedInput)} /> } } const styles = StyleSheet.create({ - percInput: { - height: '22px', - width: '50px', + // Ref: tableTd_percentage on ledgetTable.js + pinnedInput: { + width: '5ch', + border: `1px solid #c4c5c5`, borderRadius: globalStyles.radius.borderRadius, textAlign: 'right', - backgroundColor: 'transparent', + background: 'transparent', outline: 'none', - border: `1px solid #c4c5c5`, - padding: '0 9px', + padding: '0 1ch', fontSize: '16px', - marginRight: '-10px', + + // To cancel the global color setting + color: '#656565', ':focus': { - backgroundColor: '#fff', + background: '#fff', borderColor: globalStyles.color.highlightBlue } } diff --git a/app/renderer/components/styles/global.js b/app/renderer/components/styles/global.js index d3bdabce0e8..f72519ada83 100644 --- a/app/renderer/components/styles/global.js +++ b/app/renderer/components/styles/global.js @@ -348,6 +348,18 @@ const globalStyles = { fontSize: { regular: '14.5px' } + }, + + sortableTable: { + cell: { + normal: { + padding: '.6rem' + }, + + small: { + padding: '.5rem' + } + } } } diff --git a/js/about/preferences.js b/js/about/preferences.js index c8966a94c99..3c5f4fcbe62 100644 --- a/js/about/preferences.js +++ b/js/about/preferences.js @@ -251,18 +251,18 @@ class SearchSelectEntry extends ImmutableComponent { class SearchEntry extends ImmutableComponent { render () { - return
- - {this.props.name} + return
+ + {this.props.name}
} } class SearchShortcutEntry extends ImmutableComponent { render () { - return
+ return {this.props.shortcut} -
+
} } @@ -270,17 +270,10 @@ class SearchTab extends ImmutableComponent { get searchProviders () { let entries = searchProviders.providers let array = [] - const iconSize = 16 + entries.forEach((entry) => { - let iconStyle = { - backgroundImage: `url(${entry.image})`, - minWidth: iconSize, - width: iconSize, - backgroundSize: iconSize, - height: iconSize, - display: 'inline-block', - verticalAlign: 'middle' - } + let iconStyle = {backgroundImage: `url(${entry.image})`} + array.push([ { html: , @@ -311,7 +304,6 @@ class SearchTab extends ImmutableComponent { addHoverClass onClick={this.hoverCallback.bind(this)} tableClassNames={css(styles.sortableTable_searchTab)} - columnClassNames={['default', 'searchEngine', 'engineGoKey']} /> @@ -1038,6 +1030,28 @@ const styles = StyleSheet.create({ sortableTable_searchTab: { width: '704px', marginBottom: globalStyles.spacing.settingsListContainerMargin // See syncTab.js for use cases + }, + + searchEntry: { + display: 'flex', + alignItems: 'center' + }, + + searchEntry__icon: { + height: '1rem', + width: '1rem', + backgroundSize: '1rem', + + // See table__tbody__tr__td on sortableTable.js + marginRight: globalStyles.sortableTable.cell.normal.padding + }, + + searchEntry__name: { + fontSize: '1rem' + }, + + searchShortcutEntry: { + fontSize: '1rem' } }) diff --git a/less/about/preferences.less b/less/about/preferences.less index 6b495f9645c..750332ba4d4 100644 --- a/less/about/preferences.less +++ b/less/about/preferences.less @@ -184,23 +184,12 @@ input[type="checkbox"][disabled] { } table.sortableTable { + // For Sync tab tbody td:first-of-type { text-align: center; color: @braveOrange; font-weight: 800; } - - .default { - width: 95px; - } - .searchEngine { - width: 304px !important; - padding-left: 10px; - } - .engineGoKey { - padding-left: 4px; - width: 250px; - } } #searchSelectIcon { From 48674bac95bc208de9382d7a964210494abe0dd9 Mon Sep 17 00:00:00 2001 From: Suguru Hirahara Date: Wed, 27 Sep 2017 16:29:31 +0900 Subject: [PATCH 2/3] Update ledgerTable and pinnedInput to the BEM style --- .../preferences/payment/ledgerTable.js | 76 ++++++++----------- .../preferences/payment/pinnedInput.js | 6 +- 2 files changed, 35 insertions(+), 47 deletions(-) diff --git a/app/renderer/components/preferences/payment/ledgerTable.js b/app/renderer/components/preferences/payment/ledgerTable.js index b5f3dcb2b9e..682b93427c0 100644 --- a/app/renderer/components/preferences/payment/ledgerTable.js +++ b/app/renderer/components/preferences/payment/ledgerTable.js @@ -114,7 +114,7 @@ class LedgerTable extends ImmutableComponent { get columnClassNames () { return [ - css(styles.alignRight, styles.verifiedTd), // verified + css(styles.column_verified), // verified css(styles.column_sites), // sites css(styles.alignCenter), // include css(styles.alignRight), // views @@ -131,14 +131,14 @@ class LedgerTable extends ImmutableComponent { pinnedRows.map(item => { j++ return this.enabledForSite(item) - ? css(j % 2 && styles.tableTdBg) - : css(styles.paymentsDisabled, j % 2 && styles.tableTdBg) + ? css(j % 2 && styles.row) + : css(j % 2 && styles.row, styles.row_disabled) }).toJS(), unPinnedRows.map(item => { j++ return this.enabledForSite(item) - ? css(j % 2 && styles.tableTdBg) - : css(styles.paymentsDisabled, j % 2 && styles.tableTdBg) + ? css(j % 2 && styles.row) + : css(j % 2 && styles.row, styles.row_disabled) }).toJS() ] } @@ -170,10 +170,10 @@ class LedgerTable extends ImmutableComponent { { faviconURL - ? {siteName} - : + ? {siteName} + : } - {siteName} + {siteName}
, value: publisherKey @@ -188,7 +188,8 @@ class LedgerTable extends ImmutableComponent { testId='pinnedDisabled' onClick={() => {}} /> - : + html: { pinned ? 0 && styles.pinnedBody), '']} @@ -317,7 +318,7 @@ class LedgerTable extends ImmutableComponent { /> { showButton - ?
+ ?
Date: Mon, 2 Oct 2017 18:22:13 +0900 Subject: [PATCH 3/3] Improve SortableTable - Replace headerClassNames with headerStyles - Replace columnClassNames with columnStyles on ledgerTable - Replace bodyClassNames with bodyStyles - Remove customCellClassesStr - Remove .th-inner - Set white-space: nowrap to ledgerTable.js Auditors: Test Plan: 1. Run `npm run add-simulated-synopsis-visits 100` 2. Open about:preferences#payments 3. Pin some entries 4. Make sure the table is properly displayed --- .../about/bookmarks/bookmarkTitleHeader.js | 2 +- .../components/common/sortableTable.js | 31 +++++------ .../preferences/payment/ledgerTable.js | 51 ++++++++++--------- .../preferences/payment/pinnedInput.js | 3 -- less/sortableTable.less | 1 - 5 files changed, 45 insertions(+), 43 deletions(-) diff --git a/app/renderer/about/bookmarks/bookmarkTitleHeader.js b/app/renderer/about/bookmarks/bookmarkTitleHeader.js index 4fc6d0edb63..c840e309f93 100644 --- a/app/renderer/about/bookmarks/bookmarkTitleHeader.js +++ b/app/renderer/about/bookmarks/bookmarkTitleHeader.js @@ -23,7 +23,7 @@ class BookmarkTitleHeader extends ImmutableComponent { windowActions.addBookmark(newBookmark) } render () { - return
+ return
{ const content = this.generateTableRows(rows, i) return (content.length > 0) - ? + ? {content} : null @@ -487,6 +481,7 @@ class SortableTable extends React.Component { const isString = typeof heading === 'string' const sortMethod = this.sortingDisabled ? 'none' : (dataType === 'number' ? 'number' : undefined) if (isString) headerClasses['heading-' + heading] = true + return { isString - ?
+ ?
: heading } @@ -585,12 +580,18 @@ const styles = StyleSheet.create({ table__tbody__tr__td: { boxSizing: 'border-box', - padding: `calc(${globalStyles.sortableTable.cell.normal.padding} / 2.25) ${globalStyles.sortableTable.cell.normal.padding}` + paddingTop: `calc(${globalStyles.sortableTable.cell.normal.padding} / 2.25)`, + paddingBottom: `calc(${globalStyles.sortableTable.cell.normal.padding} / 2.25)`, + paddingRight: `${globalStyles.sortableTable.cell.normal.padding}`, + paddingLeft: `${globalStyles.sortableTable.cell.normal.padding}` }, // Add 'smallRow' to SortableTable to decrease padding values. table__tbody__tr_smallRow__td: { - padding: `calc(${globalStyles.sortableTable.cell.small.padding} / 2.25) ${globalStyles.sortableTable.cell.small.padding}`, + paddingTop: `calc(${globalStyles.sortableTable.cell.small.padding} / 2.25)`, + paddingBottom: `calc(${globalStyles.sortableTable.cell.small.padding} / 2.25)`, + paddingRight: `${globalStyles.sortableTable.cell.small.padding}`, + paddingLeft: `${globalStyles.sortableTable.cell.small.padding}`, height: '1.5rem' } }) diff --git a/app/renderer/components/preferences/payment/ledgerTable.js b/app/renderer/components/preferences/payment/ledgerTable.js index 682b93427c0..9797655a479 100644 --- a/app/renderer/components/preferences/payment/ledgerTable.js +++ b/app/renderer/components/preferences/payment/ledgerTable.js @@ -112,15 +112,15 @@ class LedgerTable extends ImmutableComponent { } } - get columnClassNames () { + get columnStyles () { return [ - css(styles.column_verified), // verified - css(styles.column_sites), // sites - css(styles.alignCenter), // include - css(styles.alignRight), // views - css(styles.alignRight), // time spent - css(styles.alignRight, styles.column_percentage), // percentage - css(styles.alignCenter) // actions + styles.column_verified, + styles.column_sites, + null, // include + [styles.column_nowrap, styles.alignRight], // views + [styles.column_nowrap, styles.alignRight], // time spent + [styles.alignRight, styles.column_percentage], + styles.alignCenter // actions ] } @@ -184,12 +184,14 @@ class LedgerTable extends ImmutableComponent { small disabled checkedOn + switchClassName={css(styles.switchControl_center)} indicatorClassName={css(styles.pinnedToggle)} testId='pinnedDisabled' onClick={() => {}} /> : + html: { pinned ? 0 && styles.pinnedBody), '']} + bodyStyles={[unPinnedRows.size > 0 && styles.pinnedBody, '']} onContextMenu={aboutActions.contextMenu} contextMenuName='synopsis' rowObjects={[ @@ -369,9 +371,12 @@ const styles = StyleSheet.create({ }, column_verified: { - // Set the same padding with the padding-top and padding-bottom - paddingRight: `calc(${globalStyles.sortableTable.cell.small.padding} / 2) !important`, - paddingLeft: `calc(${globalStyles.sortableTable.cell.small.padding} / 2) !important` + paddingRight: 0, + paddingLeft: 0 + }, + + column_nowrap: { + whiteSpace: 'nowrap' }, column_sites: { @@ -380,12 +385,9 @@ const styles = StyleSheet.create({ }, column_percentage: { - minWidth: '3ch' - }, - - column_percentage__percentageValue: { - // To cancel the global color setting - color: '#656565' + minWidth: '3ch', + paddingRight: `calc(${globalStyles.sortableTable.cell.small.padding} + .2rem)`, + paddingLeft: `calc(${globalStyles.sortableTable.cell.small.padding} + .2rem)` }, pinnedBody: { @@ -414,6 +416,10 @@ const styles = StyleSheet.create({ padding: '0 6px' }, + switchControl_center: { + justifyContent: 'center' + }, + hideExcludedSites: { display: 'grid', alignItems: 'center', @@ -435,8 +441,7 @@ const styles = StyleSheet.create({ }, alignCenter: { - display: 'flex', - justifyContent: 'center' + textAlign: 'center' }, actionIcons: { diff --git a/app/renderer/components/preferences/payment/pinnedInput.js b/app/renderer/components/preferences/payment/pinnedInput.js index 264412beab8..e304fe84e11 100644 --- a/app/renderer/components/preferences/payment/pinnedInput.js +++ b/app/renderer/components/preferences/payment/pinnedInput.js @@ -62,9 +62,6 @@ const styles = StyleSheet.create({ // include border width width: 'calc(5ch + 2px)', - // To cancel the global color setting - color: '#656565', - ':focus': { background: '#fff', borderColor: globalStyles.color.highlightBlue diff --git a/less/sortableTable.less b/less/sortableTable.less index 425e00e2afc..9d54e8c333e 100644 --- a/less/sortableTable.less +++ b/less/sortableTable.less @@ -25,7 +25,6 @@ table.sortableTable { } td { - color: @mediumGray; -webkit-font-smoothing: antialiased; position: relative; width: auto;