Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Syncing styling #7152

Merged
merged 1 commit into from
Feb 10, 2017
Merged

Syncing styling #7152

merged 1 commit into from
Feb 10, 2017

Conversation

ayumi
Copy link
Contributor

@ayumi ayumi commented Feb 10, 2017

also show device name

Auditors: @diracdeltas

screen shot 2017-02-09 at 18 35 13
screen shot 2017-02-09 at 18 35 35
screen shot 2017-02-09 at 18 35 41
screen shot 2017-02-09 at 18 35 52

Also show device name

Auditors: @diracdeltas
@ayumi ayumi force-pushed the feature/syncing-styling branch from f9026e6 to 7dad719 Compare February 10, 2017 02:36
@diracdeltas
Copy link
Member

[minor] the large right-side padding is a little weird
screen shot 2017-02-10 at 4 47 26 am

some of the syncing tests are unstoked https://travis-ci.org/brave/browser-laptop/builds/200218416#L4843

font-size: 18px;
font-family: monospace;
.browserButton + .browserButton {
margin-left: 0.75em;
Copy link
Contributor

@luixxiul luixxiul Feb 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be already covered with

& + .browserButton {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

5px was insufficient :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely!

@@ -56,7 +65,7 @@ class SyncTab extends ImmutableComponent {
}
return this.props.syncQRVisible
? <div>
<div><Button l10nId='syncHideQR' className='whiteButton syncToggleButton' onClick={this.props.hideQR} /></div>
<div><Button l10nId='syncHideQR' className='whiteButton wideButton syncToggleButton' onClick={this.props.hideQR} /></div>
Copy link
Contributor

@luixxiul luixxiul Feb 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need wideButton for this? the min-width of the buttons has been set globally with #6387

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I applied this class as a hack to make the "button pressed / secret visible" state have a larger button. wideButton might be inappropriate so I can explicitly set a width for these buttons.

@@ -76,7 +85,7 @@ class SyncTab extends ImmutableComponent {
]
return this.props.syncPassphraseVisible
? <div>
<Button l10nId='syncHidePassphrase' className='whiteButton syncToggleButton' onClick={this.props.hidePassphrase} />
<Button l10nId='syncHidePassphrase' className='whiteButton wideButton syncToggleButton' onClick={this.props.hidePassphrase} />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

@ayumi
Copy link
Contributor Author

ayumi commented Feb 10, 2017

@diracdeltas ran tests again which were successful. the test failures look to be intermittent and I'll try to address that in a separate PR.

Adjusted switch/name spacing based on feedback:
screen shot 2017-02-10 at 09 59 12

@diracdeltas diracdeltas merged commit 04af333 into feature/syncing Feb 10, 2017
@bsclifton bsclifton deleted the feature/syncing-styling branch May 22, 2017 16:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants