Settings: Printers: Cleanup layout and fix for long names

Settings: Printers: Cleanup layout and fix for long names

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

Review-Url: https://codereview.chromium.org/2906893003
Cr-Commit-Position: refs/heads/master@{#475665}
Committed: https://chromium.googlesource.com/chromium/src/+/e9481b6417f18f946c21e1a2049c7052caf0c8e7

[stevenjb, 2017-05-26 16:38:49]

Description was changed from

==========
Settings: Printers: Cleanup layout and fix for long names

BUG=726676
==========

to

==========
Settings: Printers: Cleanup layout and fix for long names

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

[stevenjb, 2017-05-26 16:43:05]

stevenjb@chromium.org changed reviewers:
+ xdai@chromium.org

[stevenjb, 2017-05-26 16:43:05]

There were a couple of problems with the layout:
* We need .text-elide (in settings_shared_css.html) to elide long names
properly.
* id tags were used inside a dom-repeat which would create multiple items with
the same id which can be a problem.

[xdai1, 2017-05-26 17:04:27]

On 2017/05/26 16:43:05, stevenjb wrote:
> There were a couple of problems with the layout:
> * We need .text-elide (in settings_shared_css.html) to elide long names
> properly.
> * id tags were used inside a dom-repeat which would create multiple items with
> the same id which can be a problem.

Thanks for fixing it. I patched your CL locally and found the layout of the page
changed from https://screenshot.googleplex.com/h8MPX5OCDsv to
https://screenshot.googleplex.com/irCSfpG7mSw. Is this intentional?

[stevenjb, 2017-05-26 19:32:11]

Mostly intentional, thanks for grabbing the screenshots, I realized that their
shouldn't be a final bottom border and remembered that we have 'vertical-list'
for that.

(In general we should do as little per-page custom CSS as possible, but it can
be tricky to find the right classes to use).

Done!

[stevenjb, 2017-05-26 19:32:12]

Mostly intentional, thanks for grabbing the screenshots, I realized that their
shouldn't be a final bottom border and remembered that we have 'vertical-list'
for that.

(In general we should do as little per-page custom CSS as possible, but it can
be tricky to find the right classes to use).

Done!

[xdai1, 2017-05-26 22:18:04]

On 2017/05/26 19:32:12, stevenjb wrote:
> Mostly intentional, thanks for grabbing the screenshots, I realized that their
> shouldn't be a final bottom border and remembered that we have 'vertical-list'
> for that.
> 
> (In general we should do as little per-page custom CSS as possible, but it can
> be tricky to find the right classes to use).
> 
> Done!

lgtm Thanks for fixing it!

FYI: I just talked with weifangsun@, the PM of this project. She didn't have
objection to the UI change. But she wanted the UX person to be aware of this
change. I'll upload the screenshots in the bug and cc the UX designer.

[stevenjb, 2017-05-30 17:00:27]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-05-30 17:00:58]

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-05-30 19:08:10]

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

[commit-bot: I haz the power, 2017-05-30 19:08:14]

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

[stevenjb, 2017-05-30 19:59:08]

The CQ bit was checked by stevenjb@chromium.org

[commit-bot: I haz the power, 2017-05-30 19:59:46]

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-05-30 20:57:13]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1496174348217140,
"parent_rev": "c95caf021bebd3bc58dafc206ff9f43678dc8d73", "commit_rev":
"e9481b6417f18f946c21e1a2049c7052caf0c8e7"}

[commit-bot: I haz the power, 2017-05-30 20:57:21]

Description was changed from

==========
Settings: Printers: Cleanup layout and fix for long names

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

to

==========
Settings: Printers: Cleanup layout and fix for long names

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

Review-Url: https://codereview.chromium.org/2906893003
Cr-Commit-Position: refs/heads/master@{#475665}
Committed:
https://chromium.googlesource.com/chromium/src/+/e9481b6417f18f946c21e1a2049c...
==========

[commit-bot: I haz the power, 2017-05-30 20:57:23]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/e9481b6417f18f946c21e1a2049c...