Fix button spacing for overriding variations country.

Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/4168a20e872bf472b0febb413d625d39ef0ee6d0
Cr-Commit-Position: refs/heads/master@{#425039}

[hamelphi, 2016-10-05 20:42:39]

Description was changed from

==========
Fix button spacing for overriding variations country.

BUG=614961
==========

to

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[hamelphi, 2016-10-05 21:13:33]

hamelphi@chromium.org changed reviewers:
+ asvitkine@chromium.org

[Alexei Svitkine (slow), 2016-10-05 21:50:44]

lgtm

[hamelphi, 2016-10-05 21:58:09]

The CQ bit was checked by hamelphi@chromium.org

[commit-bot: I haz the power, 2016-10-05 21:59:00]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-05 22:10:05]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-10-05 22:10:06]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[hamelphi, 2016-10-06 14:22:03]

hamelphi@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2016-10-10 02:48:26]

Description was changed from

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[groby-ooo-7-16, 2016-10-10 02:48:26]

groby@chromium.org changed reviewers:
+ dpapad@chromium.org
- groby@chromium.org

[groby-ooo-7-16, 2016-10-10 02:48:57]

groby@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2016-10-10 02:48:58]

groby@chromium.org changed reviewers:
+ groby@chromium.org

[groby-ooo-7-16, 2016-10-10 02:48:58]

Handing off review to dpapad, who's more familiar with WebUI. (Off the cuff, the
addition of an extra class looks strange to me, though)

[groby-ooo-7-16, 2016-10-10 02:48:58]

Handing off review to dpapad, who's more familiar with WebUI. (Off the cuff, the
addition of an extra class looks strange to me, though)

[dpapad, 2016-10-10 17:30:53]

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
File chrome/browser/resources/translate_internals/prefs.html (right):

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
chrome/browser/resources/translate_internals/prefs.html:46: <p
id="country-override" class="country-override"></p>
The CSS class is never used. Can we revert this?

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
File chrome/browser/resources/translate_internals/translate_internals.css
(right):

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
chrome/browser/resources/translate_internals/translate_internals.css:142:
#country-override br {
This is a very indirect way to add spacing between the textbox and the button.
Can we simply used something like

#country-override button {
 margin-top: 10px;
}

OR

#country-override input {
  margin-bottom: 10px;
}

[hamelphi, 2016-10-11 15:54:42]

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
File chrome/browser/resources/translate_internals/prefs.html (right):

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
chrome/browser/resources/translate_internals/prefs.html:46: <p
id="country-override" class="country-override"></p>
On 2016/10/10 17:30:52, dpapad wrote:
> The CSS class is never used. Can we revert this?

Done.

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
File chrome/browser/resources/translate_internals/translate_internals.css
(right):

https://codereview.chromium.org/2396963002/diff/1/chrome/browser/resources/tr...
chrome/browser/resources/translate_internals/translate_internals.css:142:
#country-override br {
On 2016/10/10 17:30:52, dpapad wrote:
> This is a very indirect way to add spacing between the textbox and the button.
> Can we simply used something like
> 
> #country-override button {
>  margin-top: 10px;
> }
> 
> OR
> 
> #country-override input {
>   margin-bottom: 10px;
> }

Done.

[dpapad, 2016-10-11 18:00:37]

LG with question.

https://codereview.chromium.org/2396963002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/translate_internals/translate_internals.css
(right):

https://codereview.chromium.org/2396963002/diff/20001/chrome/browser/resource...
chrome/browser/resources/translate_internals/translate_internals.css:143:
margin-bottom: 1px;
Is 1px enough spacing according to the bug?

[dpapad, 2016-10-11 18:00:51]

lgtm

[hamelphi, 2016-10-12 21:41:00]

hamelphi@chromium.org changed reviewers:
- groby@chromium.org

[hamelphi, 2016-10-12 21:41:00]

https://codereview.chromium.org/2396963002/diff/20001/chrome/browser/resource...
File chrome/browser/resources/translate_internals/translate_internals.css
(right):

https://codereview.chromium.org/2396963002/diff/20001/chrome/browser/resource...
chrome/browser/resources/translate_internals/translate_internals.css:143:
margin-bottom: 1px;
On 2016/10/11 18:00:37, dpapad wrote:
> Is 1px enough spacing according to the bug?

I believe so. I updated the bug with new screenshots.
https://bugs.chromium.org/p/chromium/issues/detail?id=614961

[hamelphi, 2016-10-13 14:40:16]

The CQ bit was checked by hamelphi@chromium.org

[hamelphi, 2016-10-13 14:40:16]

The patchset sent to the CQ was uploaded after l-g-t-m from
asvitkine@chromium.org
Link to the patchset: https://codereview.chromium.org/2396963002/#ps20001
(title: "Use pixel offset.")

[commit-bot: I haz the power, 2016-10-13 14:40:26]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-13 15:34:04]

Description was changed from

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[commit-bot: I haz the power, 2016-10-13 15:34:06]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-10-13 15:38:19]

Description was changed from

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Fix button spacing for overriding variations country.

BUG=614961
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Committed: https://crrev.com/4168a20e872bf472b0febb413d625d39ef0ee6d0
Cr-Commit-Position: refs/heads/master@{#425039}
==========

[commit-bot: I haz the power, 2016-10-13 15:38:20]

Patchset 2 (id:??) landed as
https://crrev.com/4168a20e872bf472b0febb413d625d39ef0ee6d0
Cr-Commit-Position: refs/heads/master@{#425039}