Separate input and select styles using Aphrodite #6689

bsclifton · 2017-01-16T22:42:06Z

Submitted a ticket for my issue if one did not already exist.
Used Github auto-closing keywords in the commit message.
Added/updated tests for this change (for new code or code which already has tests).
Ran git rebase -i to squash commits (if needed).

This is an Aphrodite friendly redo of #6084

I tested each area manually and did my best to compare to the original PR. @luixxiul, please let me know what you think. I did omit a two updates to select (because they looked bad):

select box font being 1em
select box having margin 0 5px

screenshots of textbox / dropdown

Updates to about:styles

luixxiul · 2017-01-17T02:21:33Z

app/renderer/components/textbox.js

+}
+
+const styles = StyleSheet.create({
+  'formControl': {


Can we share this with that of app/renderer/components/dropdown.js, setting globally somewhere and importing it? I'm wondering if setting them in each file would make a possibility of style inconsistency.

@luixxiul we sure can; implemented this feedback with 46d07ec

luixxiul · 2017-01-17T05:23:47Z

https://travis-ci.org/brave/browser-laptop/builds/192513240#L4619

SyntaxError: /home/travis/build/brave/browser-laptop/app/extensions/brave/img/caret_down_grey.svg: Unexpected token, expected } (1:90)

luixxiul · 2017-01-17T08:54:52Z

app/renderer/components/styles/commonStyles.js

+const {StyleSheet} = require('aphrodite')
+const globalStyles = require('./global')
+
+const styles = StyleSheet.create({


How about adding these under this line for reference? Is it too much?

/* app/renderer/components/dropdown.js app/renderer/components/textbox.js */

luixxiul · 2017-01-17T16:32:45Z

Also I think we can remove some properties in LESS files thanks to the change. I'll mention them as below for a cleanup task.

luixxiul · 2017-01-17T16:32:56Z

app/renderer/components/styles/commonStyles.js

+const globalStyles = require('./global')
+
+const styles = StyleSheet.create({
+  'formControl': {


browser-laptop/less/forms.less

Lines 8 to 21 in 3d1db35

.form-control {

display: block;

background: white;

border: solid 1px @lightGray;

border-radius: @borderRadius;

box-shadow: inset 0 1px 1px rgba(0,0,0,.075);

box-sizing: border-box;

color: @darkGray;

font-size: 14.5px;

height: 2.25em;

outline: none;

padding: 0.4em;

width: 100%;

}

luixxiul · 2017-01-17T16:35:25Z

app/renderer/components/textbox.js

+    }
+  },
+  'isSettings': {
+    width: '280px'


browser-laptop/less/about/preferences.less

Line 373 in baff549

width: 280px;

luixxiul · 2017-01-17T16:37:39Z

app/renderer/components/textbox.js

+    boxShadow: 'none',
+    outline: 'none'
+  },
+  'recoveryKeys': {


browser-laptop/less/about/preferences.less

Lines 724 to 727 in baff549

.firstRecoveryKey,

.secondRecoveryKey {

margin-bottom: 20px;

}

luixxiul · 2017-01-17T16:44:11Z

js/about/preferences.js

@@ -1248,16 +1219,17 @@ class PaymentsTab extends ImmutableComponent {
              <td>
                <SettingsList>
                  <SettingItem>
-                    <select className='form-control fundsSelectBox'
+                    <FormDropdown


@bsclifton Is this applied in some way?

no; I manually tested and it doesn't do anything (it still stays ~10px). Also, the "Add funds" button has a similar padding. If you remove padding from "Add funds" button (ex: set to 0), it looks bad

luixxiul · 2017-01-17T16:53:56Z

Feedbacks left.

bsclifton · 2017-01-23T17:22:55Z

@luixxiul I removed styles per your feedback; also I updated the Shields panel to use this new style:

@bbondy

Fixes #5972 - Create aphrodite components for "text" input type - Create aphrodite components for "select" input type - Small LESS updates to primary button (not Aphrodite component yet) - Update about:styles Auditors: @bbondy, @luixxiul, @cezaraugusto, @bradleyrichter

@luixxiul

Implement feedback from #6689 (review) Auditors: @luixxiul

@luixxiul

… panel to use new styles - Updated selectors for testing shields - added padding to dropdown to cover size of caret - Fixed broken navigationBar test Auditors: @luixxiul

@luixxiul

Implement feedback from #6689 (review) Auditors: @luixxiul

luixxiul · 2017-01-25T04:01:30Z

I think the SVG merged with this PR is causing an error on Travis:

9) Preferences component "before all" hook:
     SyntaxError: /home/travis/build/brave/browser-laptop/app/extensions/brave/img/caret_down_grey.svg: Unexpected token, expected } (1:90)
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32.86 17.81"><defs><style>.cls-1{fill:gray;}</style></defs><title>caret_down_grey</title><g id="Layer_2" data-name="Layer 2"><g id="Layer_1_copy" data-name="Layer 1 copy"><path class="cls-1" d="M15.3,17.35.49,2.77A1.62,1.62,0,0,1,1.62,0H31.24a1.62,1.62,0,0,1,1.14,2.77L17.57,17.35A1.62,1.62,0,0,1,15.3,17.35Z"/></g></g></svg>

bsclifton · 2017-01-25T04:58:23Z

@luixxiul you are correct; I'm going to need some help w/ it. Let me ask around on Slack

@luixxiul

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

@luixxiul

Implement feedback from #6689 (review) Auditors: @luixxiul

@luixxiul

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

@luixxiul

Implement feedback from #6689 (review) Auditors: @luixxiul

@luixxiul

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

@luixxiul

Implement feedback from #6689 (review) Auditors: @luixxiul

@luixxiul

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

bsclifton added the design A design change, especially one which needs input from the design team. label Jan 16, 2017

bsclifton added this to the 0.13.1 milestone Jan 16, 2017

bsclifton self-assigned this Jan 16, 2017

bsclifton requested review from bbondy, luixxiul, cezaraugusto and bradleyrichter January 16, 2017 22:42

bsclifton changed the title ~~Fix 5972 aphrodite~~ Separate input and select styles using Aphrodite Jan 16, 2017

luixxiul reviewed Jan 17, 2017

View reviewed changes

luixxiul added refactoring refactoring/aphrodite and removed refactoring labels Jan 17, 2017

luixxiul reviewed Jan 17, 2017

View reviewed changes

luixxiul mentioned this pull request Jan 20, 2017

Replace input and select elements with <FormTextbox> and <FormDropdown> in .bookmarkFormTable #6777

Closed

bsclifton changed the base branch from master to 0.13.1-branch January 23, 2017 17:21

bsclifton added 3 commits January 23, 2017 12:16

Pulled common styles into a file to be shared w/ Textbox + Dropdown

fa06ec2

Implement feedback from #6689 (review) Auditors: @luixxiul

Style cleanup per recommendation from @luixxiul. Also updated Shields…

f276467

… panel to use new styles - Updated selectors for testing shields - added padding to dropdown to cover size of caret - Fixed broken navigationBar test Auditors: @luixxiul

bsclifton merged commit e69d86f into brave:0.13.1-branch Jan 23, 2017

bsclifton deleted the fix-5972-aphrodite branch January 23, 2017 19:17

This was referenced Jan 23, 2017

Separate input and select styles using Aphrodite #5972

Closed

Update: Keep tabs same size when closing (fixes #6088) #6342

Merged

bsclifton added a commit that referenced this pull request Jan 24, 2017

Pulled common styles into a file to be shared w/ Textbox + Dropdown

0ce92fa

Implement feedback from #6689 (review) Auditors: @luixxiul

bsclifton added a commit that referenced this pull request Jan 25, 2017

Fix test that was accidentally broke when introducing Aphrodite compo…

6d3ed6d

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

bsclifton added a commit that referenced this pull request Jan 25, 2017

Pulled common styles into a file to be shared w/ Textbox + Dropdown

7739714

Implement feedback from #6689 (review) Auditors: @luixxiul

bsclifton added a commit that referenced this pull request Jan 25, 2017

Fix test that was accidentally broke when introducing Aphrodite compo…

0fb932c

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

bsclifton added a commit that referenced this pull request Jan 25, 2017

Pulled common styles into a file to be shared w/ Textbox + Dropdown

fc93bd6

Implement feedback from #6689 (review) Auditors: @luixxiul

bsclifton added a commit that referenced this pull request Jan 25, 2017

Fix test that was accidentally broke when introducing Aphrodite compo…

7239b48

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

bsclifton added a commit that referenced this pull request Jan 26, 2017

Pulled common styles into a file to be shared w/ Textbox + Dropdown

662b4e4

Implement feedback from #6689 (review) Auditors: @luixxiul

bsclifton added a commit that referenced this pull request Jan 26, 2017

Fix test that was accidentally broke when introducing Aphrodite compo…

4ad23fa

…nents for dropdown/textbox (broke when #6689 was merged) Auditors: @luixxiul, @bbondy

bsclifton referenced this pull request Jan 30, 2017

adjusted box-shadow style for dropdown, and added Johnny Walker.

a14fcdf

luixxiul mentioned this pull request Feb 1, 2017

Shield menu drop down has scroll bars #6959

Closed

This was referenced Feb 3, 2017

Manual Tests for Windows x64 0.13.2 RC2 #7030

Closed

Manual Tests for Windows ia-32 0.13.2 RC3 #7029

Closed

Manual Tests for Linux 0.13.2 RC3 #7028

Closed

Manual Tests for OS X 0.13.2 RC2 #7027

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate input and select styles using Aphrodite #6689

Separate input and select styles using Aphrodite #6689

bsclifton commented Jan 16, 2017 •

edited

Loading

luixxiul Jan 17, 2017 •

edited

Loading

bsclifton Jan 17, 2017

luixxiul commented Jan 17, 2017

luixxiul Jan 17, 2017 •

edited

Loading

luixxiul commented Jan 17, 2017 •

edited

Loading

luixxiul Jan 17, 2017 •

edited

Loading

luixxiul Jan 17, 2017

luixxiul Jan 17, 2017

luixxiul Jan 17, 2017 •

edited

Loading

bsclifton Jan 23, 2017

luixxiul commented Jan 17, 2017

bsclifton commented Jan 23, 2017

luixxiul commented Jan 25, 2017

bsclifton commented Jan 25, 2017

	.form-control {
	display: block;
	background: white;
	border: solid 1px @lightGray;
	border-radius: @borderRadius;
	box-shadow: inset 0 1px 1px rgba(0,0,0,.075);
	box-sizing: border-box;
	color: @darkGray;
	font-size: 14.5px;
	height: 2.25em;
	outline: none;
	padding: 0.4em;
	width: 100%;
	}

Separate input and select styles using Aphrodite #6689

Separate input and select styles using Aphrodite #6689

Conversation

bsclifton commented Jan 16, 2017 • edited Loading

screenshots of textbox / dropdown

Updates to about:styles

luixxiul Jan 17, 2017 • edited Loading

Choose a reason for hiding this comment

bsclifton Jan 17, 2017

Choose a reason for hiding this comment

luixxiul commented Jan 17, 2017

luixxiul Jan 17, 2017 • edited Loading

Choose a reason for hiding this comment

luixxiul commented Jan 17, 2017 • edited Loading

luixxiul Jan 17, 2017 • edited Loading

Choose a reason for hiding this comment

luixxiul Jan 17, 2017

Choose a reason for hiding this comment

luixxiul Jan 17, 2017

Choose a reason for hiding this comment

luixxiul Jan 17, 2017 • edited Loading

Choose a reason for hiding this comment

bsclifton Jan 23, 2017

Choose a reason for hiding this comment

luixxiul commented Jan 17, 2017

bsclifton commented Jan 23, 2017

luixxiul commented Jan 25, 2017

bsclifton commented Jan 25, 2017

bsclifton commented Jan 16, 2017 •

edited

Loading

luixxiul Jan 17, 2017 •

edited

Loading

luixxiul Jan 17, 2017 •

edited

Loading

luixxiul commented Jan 17, 2017 •

edited

Loading

luixxiul Jan 17, 2017 •

edited

Loading

luixxiul Jan 17, 2017 •

edited

Loading