Change the selectors position in Save as dialog and make it focusable.

Change the selectors position in Save as dialog and make it focusable.

BUG=661460
TEST=manually tested
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795953002
Cr-Commit-Position: refs/heads/master@{#461973}
Committed: https://chromium.googlesource.com/chromium/src/+/49e876ae7c44c572607a55bdcc2ef8e992bb3f58

[tetsui2, 2017-04-04 05:18:37]

Description was changed from

==========
Change the selectors position in Save as dialog and make it focusable.

BUG=661460
TEST=manually tested
==========

to

==========
Change the selectors position in Save as dialog and make it focusable.

BUG=661460
TEST=manually tested
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

[tetsui2, 2017-04-04 05:20:02]

tetsui@google.com changed reviewers:
+ fukino@chromium.org

[tetsui2, 2017-04-04 05:20:03]

PTAL

[fukino, 2017-04-04 05:29:34]

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/foreground/css/file_manager.css:811: margin: 0 0 0
16px;
When we specify the value like this, we will have 16px left margin, but in RtL
language, we want to have 16px right margin.
(in RtL lauguage, the UI is basically "mirrored". You can see it by setting the
language from chrome://settings to, for instance, Arabic)

To avoid the difference, let's use --webkit-margin-start and
--webkit-margin-end.
(Although the previous margin 0 32px 0 36px was already broken, it's good time
to fix it)

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/main.html (right):

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/main.html:264: <paper-button
id="cancel-selection-button" tabindex="8"
Could you update go/filesapp-tabindex?

According to go/filesapp-tabindex, the "8" is already taken, so we'll need to
update more tabindexes.

[tetsui2, 2017-04-05 01:49:42]

PTAL

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/foreground/css/file_manager.css:811: margin: 0 0 0
16px;
On 2017/04/04 05:29:34, fukino wrote:
> When we specify the value like this, we will have 16px left margin, but in RtL
> language, we want to have 16px right margin.
> (in RtL lauguage, the UI is basically "mirrored". You can see it by setting
the
> language from chrome://settings to, for instance, Arabic)
> 
> To avoid the difference, let's use --webkit-margin-start and
> --webkit-margin-end.
> (Although the previous margin 0 32px 0 36px was already broken, it's good time
> to fix it)

Done.

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
File ui/file_manager/file_manager/main.html (right):

https://codereview.chromium.org/2795953002/diff/1/ui/file_manager/file_manage...
ui/file_manager/file_manager/main.html:264: <paper-button
id="cancel-selection-button" tabindex="8"
On 2017/04/04 05:29:34, fukino wrote:
> Could you update go/filesapp-tabindex?
> 
> According to go/filesapp-tabindex, the "8" is already taken, so we'll need to
> update more tabindexes.

Done.

[fukino, 2017-04-05 01:52:12]

https://codereview.chromium.org/2795953002/diff/20001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2795953002/diff/20001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/css/file_manager.css:801:
-webkit-margin-start: 16px;
nit: Sort properties alphabetically.
https://www.chromium.org/developers/web-development-style-guide says:
Alphabetize properties.
  -webkit properties should be listed at the top, sorted alphabetically.

[tetsui2, 2017-04-05 02:00:04]

Thank you for reviewing!

https://codereview.chromium.org/2795953002/diff/20001/ui/file_manager/file_ma...
File ui/file_manager/file_manager/foreground/css/file_manager.css (right):

https://codereview.chromium.org/2795953002/diff/20001/ui/file_manager/file_ma...
ui/file_manager/file_manager/foreground/css/file_manager.css:801:
-webkit-margin-start: 16px;
On 2017/04/05 01:52:12, fukino wrote:
> nit: Sort properties alphabetically.
> https://www.chromium.org/developers/web-development-style-guide says:
> Alphabetize properties.
>   -webkit properties should be listed at the top, sorted alphabetically.

Done.

[fukino, 2017-04-05 02:04:38]

lgtm

[tetsui2, 2017-04-05 02:05:01]

The CQ bit was checked by tetsui@google.com

[commit-bot: I haz the power, 2017-04-05 02:05:51]

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, 2017-04-05 02:22:01]

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

[commit-bot: I haz the power, 2017-04-05 02:22:02]

Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)

[tetsui2, 2017-04-05 03:41:48]

The CQ bit was checked by tetsui@google.com

[commit-bot: I haz the power, 2017-04-05 03:42:11]

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, 2017-04-05 03:57:25]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491363708911930,
"parent_rev": "0af77d69187ee8a529072cb3995bd111aaed5864", "commit_rev":
"49e876ae7c44c572607a55bdcc2ef8e992bb3f58"}

[commit-bot: I haz the power, 2017-04-05 03:58:35]

Description was changed from

==========
Change the selectors position in Save as dialog and make it focusable.

BUG=661460
TEST=manually tested
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation
==========

to

==========
Change the selectors position in Save as dialog and make it focusable.

BUG=661460
TEST=manually tested
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:closure_compilation

Review-Url: https://codereview.chromium.org/2795953002
Cr-Commit-Position: refs/heads/master@{#461973}
Committed:
https://chromium.googlesource.com/chromium/src/+/49e876ae7c44c572607a55bdcc2e...
==========

[commit-bot: I haz the power, 2017-04-05 03:58:36]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/49e876ae7c44c572607a55bdcc2e...