Skip to content

Commit 3e64d88

Browse files
Suguru Hiraharadfperry5
Suguru Hirahara
authored andcommitted
Refactor table.sortableTable
Addresses brave#10263 Auditors: Test Plan: 1. Open about:preferences#search 2. Make sure the custom width is applied to the table 3. Open about:preferences#sync 4. Make sure the custom width is applied to the table 5. Open about:preferences#extensions 6. Make sure '-webkit-fill-available' is applied to the table
1 parent 43394a6 commit 3e64d88

File tree

7 files changed

+54
-44
lines changed

7 files changed

+54
-44
lines changed

app/renderer/components/common/sortableTable.js

+23-2
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ const React = require('react')
66
const Immutable = require('immutable')
77
const tableSort = require('tablesort')
88

9+
const {StyleSheet, css} = require('aphrodite/no-important')
10+
911
// Utils
1012
const cx = require('../../../../js/lib/classSet')
1113
const eventUtil = require('../../../../js/lib/eventUtil')
@@ -418,16 +420,19 @@ class SortableTable extends React.Component {
418420
return <tbody>{this.generateTableRows(this.props.rows)}</tbody>
419421
}
420422
}
423+
421424
render () {
422425
if (!this.props.headings || !this.props.rows) {
423426
return false
424427
}
428+
425429
return <table
426430
{...this.getTableAttributes()}
427431
className={cx({
428432
sort: !this.sortingDisabled,
429-
sortableTable: !this.props.overrideDefaultStyle,
430-
[this.props.tableClassNames]: !!this.props.tableClassNames
433+
sortableTable: true,
434+
[this.props.tableClassNames]: !!this.props.tableClassNames,
435+
[css(styles.table, this.props.fillAvailable && styles.table_fillAvailable)]: true
431436
})}
432437
ref={(node) => { this.table = node }}>
433438
<thead>
@@ -469,4 +474,20 @@ class SortableTable extends React.Component {
469474
}
470475
}
471476

477+
const styles = StyleSheet.create({
478+
// By default the width and margin are not specified.
479+
// It can be specified by setting css to tableClassNames.
480+
// See 'styles.devices__devicesList' on syncTab.js for example.
481+
table: {
482+
boxSizing: 'border-box',
483+
cursor: 'default',
484+
borderSpacing: 0
485+
},
486+
487+
// Setting 'fillAvailable' maximizes the width of the table.
488+
table_fillAvailable: {
489+
width: '-webkit-fill-available'
490+
}
491+
})
492+
472493
module.exports = SortableTable

app/renderer/components/preferences/extensionsTab.js

+13-19
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ class ExtensionsTab extends ImmutableComponent {
5656

5757
return [
5858
{ // Icon
59-
html: <img className={css(styles.table__row__column__icon)} src={this.getIcon(extension)} />
59+
html: <img className={css(styles.tableRow__column__icon)} src={this.getIcon(extension)} />
6060
},
6161
{ // Name
6262
html: <span data-extension-id={extension.get('id')}
@@ -91,12 +91,12 @@ class ExtensionsTab extends ImmutableComponent {
9191

9292
get columnClassNames () {
9393
return [
94-
css(styles.table__row__column, styles.table__row__column_center),
95-
css(styles.table__row__column),
96-
css(styles.table__row__column),
97-
css(styles.table__row__column),
98-
css(styles.table__row__column, styles.table__row__column_center)
99-
// css(styles.table__row__column, styles.table__row__column_center)
94+
css(styles.tableRow__column, styles.tableRow__column_center),
95+
css(styles.tableRow__column),
96+
css(styles.tableRow__column),
97+
css(styles.tableRow__column),
98+
css(styles.tableRow__column, styles.tableRow__column_center)
99+
// css(styles.tableRow__column, styles.tableRow__column_center)
100100
]
101101
}
102102

@@ -107,12 +107,12 @@ class ExtensionsTab extends ImmutableComponent {
107107
return <section>
108108
<DefaultSectionTitle data-l10n-id='extensions' />
109109
<SortableTable
110+
fillAvailable
110111
sortingDisabled
111-
tableClassNames={css(styles.table)}
112112
headings={['icon', 'name', 'description', 'version', 'enabled'] /* 'exclude' */}
113113
columnClassNames={this.columnClassNames}
114114
rowClassNames={
115-
this.props.extensions.map(entry => css(styles.table__row)).toJS()
115+
this.props.extensions.map(entry => css(styles.tableRow)).toJS()
116116
}
117117
rows={this.props.extensions.map(entry => this.getRow(entry))} />
118118
<footer className={css(styles.moreInfo)}>
@@ -129,13 +129,7 @@ class ExtensionsTab extends ImmutableComponent {
129129
}
130130

131131
const styles = StyleSheet.create({
132-
table: {
133-
// TODO (Suguru): refactor sortableTable.js to remove !important
134-
width: '100% !important',
135-
marginBottom: '0 !important'
136-
},
137-
138-
table__row: {
132+
tableRow: {
139133
fontSize: '15px',
140134
background: '#fff',
141135

@@ -151,15 +145,15 @@ const styles = StyleSheet.create({
151145
}
152146
},
153147

154-
table__row__column: {
148+
tableRow__column: {
155149
padding: '0 8px'
156150
},
157151

158-
table__row__column_center: {
152+
tableRow__column_center: {
159153
textAlign: 'center'
160154
},
161155

162-
table__row__column__icon: {
156+
tableRow__column__icon: {
163157
display: 'flex',
164158
margin: 'auto',
165159
width: '32px',

app/renderer/components/preferences/payment/ledgerTable.js

+1-8
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ class LedgerTable extends ImmutableComponent {
273273
</div>
274274
</div>
275275
<SortableTable
276-
tableClassNames={css(styles.tableClass)}
276+
fillAvailable
277277
headings={['', 'publisher', 'include', 'views', 'timeSpent', 'percentage', 'actions']}
278278
defaultHeading='percentage'
279279
defaultHeadingSortOrder='desc'
@@ -339,13 +339,6 @@ const styles = StyleSheet.create({
339339
}
340340
},
341341

342-
tableClass: {
343-
width: '100%',
344-
borderCollapse: 'collapse',
345-
border: 'none',
346-
margin: '0 auto'
347-
},
348-
349342
tableTh: {
350343
color: paymentStylesVariables.tableHeader.fontColor,
351344
fontSize: '14px',

app/renderer/components/preferences/syncTab.js

+3-5
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,8 @@ class SyncTab extends ImmutableComponent {
159159
defaultHeading='syncDeviceLastActive'
160160
defaultHeadingSortOrder='desc'
161161
rows={this.devicesTableRows}
162-
customCellClasses={css(styles.devices__devicesListCell)}
163162
tableClassNames={css(styles.devices__devicesList)}
163+
customCellClasses={css(styles.devices__devicesListCell)}
164164
/>
165165
</section>
166166
}
@@ -590,10 +590,8 @@ const styles = StyleSheet.create({
590590
},
591591

592592
devices__devicesList: {
593-
// TODO: refactor sortableTable to remove !important
594-
marginBottom: `${globalStyles.spacing.dialogInsideMargin} !important`,
595-
boxSizing: 'border-box',
596-
width: '600px !important'
593+
marginBottom: globalStyles.spacing.dialogInsideMargin,
594+
width: '600px'
597595
},
598596
devices__devicesListCell: {
599597
padding: '4px 8px'

js/about/preferences.js

+14-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ const React = require('react')
77
const ImmutableComponent = require('../../app/renderer/components/immutableComponent')
88
const Immutable = require('immutable')
99
const UrlUtil = require('../lib/urlutil')
10-
const {css} = require('aphrodite/no-important')
10+
const {StyleSheet, css} = require('aphrodite/no-important')
11+
const globalStyles = require('../../app/renderer/components/styles/global')
1112
const commonStyles = require('../../app/renderer/components/styles/commonStyles')
1213

1314
// Components
@@ -323,8 +324,11 @@ class SearchTab extends ImmutableComponent {
323324
<DefaultSectionTitle data-test-id='searchSettings' data-l10n-id='searchSettings' />
324325
<SortableTable headings={['default', 'searchEngine', 'engineGoKey']} rows={this.searchProviders}
325326
defaultHeading='searchEngine'
326-
addHoverClass onClick={this.hoverCallback.bind(this)}
327-
columnClassNames={['default', 'searchEngine', 'engineGoKey']} />
327+
addHoverClass
328+
onClick={this.hoverCallback.bind(this)}
329+
tableClassNames={css(styles.sortableTable_searchTab)}
330+
columnClassNames={['default', 'searchEngine', 'engineGoKey']}
331+
/>
328332
<DefaultSectionTitle data-l10n-id='locationBarSettings' />
329333
<SettingsList>
330334
<SettingCheckbox dataL10nId='showOpenedTabMatches' prefKey={settings.OPENED_TAB_SUGGESTIONS} settings={this.props.settings} onChangeSetting={this.props.onChangeSetting} />
@@ -941,6 +945,13 @@ class AboutPreferences extends React.Component {
941945
}
942946
}
943947

948+
const styles = StyleSheet.create({
949+
sortableTable_searchTab: {
950+
width: '704px',
951+
marginBottom: globalStyles.spacing.settingsListContainerMargin // See syncTab.js for use cases
952+
}
953+
})
954+
944955
module.exports = {
945956
AboutPreferences: <AboutPreferences />
946957
}

less/about/preferences.less

-2
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,6 @@ input[type="checkbox"][disabled] {
247247
}
248248

249249
table.sortableTable {
250-
width: 704px;
251-
252250
tbody td:first-of-type {
253251
text-align: center;
254252
color: @braveOrange;

less/sortableTable.less

-5
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,6 @@ table.sort {
1515
}
1616

1717
table.sortableTable {
18-
width: 100%;
19-
margin-bottom: 40px;
20-
cursor: default;
21-
border-spacing: 0;
22-
2318
tr {
2419
height: 1.7em;
2520
padding: 5px 20px;

0 commit comments

Comments
 (0)