bluetooth: Add sidebar and page manager for chrome://bluetooth-internals.

chrome/browser/resources/bluetooth_internals/bluetooth_internals.css

Index: chrome/browser/resources/bluetooth_internals/bluetooth_internals.css
diff --git a/chrome/browser/resources/bluetooth_internals/bluetooth_internals.css b/chrome/browser/resources/bluetooth_internals/bluetooth_internals.css
index 0f4812e4303665280190b0d205c59f33f3801c0c..c2a53e17693d6e169cbe1ae2469d55304871f255 100644
--- a/chrome/browser/resources/bluetooth_internals/bluetooth_internals.css
+++ b/chrome/browser/resources/bluetooth_internals/bluetooth_internals.css
@@ -13,83 +13,223 @@ body {
   padding: 0;
 }
 
+h1 {
+    -webkit-margin-start: 23px;
+    color: rgb(92, 97, 102);
+    margin-bottom: 1em;
+    margin-top: 21px;

[ortuno, 2016/12/05 05:45:28]

Some of these values seem pretty random. Any reason why we are not reusing the
ones that about:inspect uses?

[mbrunson, 2016/12/06 00:25:45]

I was using chrome://settings's CSS as a guide. Each internal page looks similar
but they all have their own stylesheets, so the values and configuration aren't
exactly the same. There is no one truth here unfortunately so I'm partially
copying and partially making it mobile friendly.

Some of these rules are overlapping and could be combined/removed.

[ortuno, 2016/12/06 10:16:04]

:( hopefully we share more with polymer.

+}

[ortuno, 2016/12/05 05:45:28]

There are two types of headers: Sidebar and Page.

We have the following rule:

#page-container header > h1 {
    margin: 0;
    padding: 21px 0 13px;
}

which means the only property from the first rule that applies is color. I think
the following would be better:

h1 {
  color: rgb(92, 97, 102);
}

.sidebar-content > header > h1 {
  -webkit-margin-start: 23px;
  margin-bottom: 1em;
  margin-top: 21px;
}

[mbrunson, 2016/12/06 00:25:45]

Done.

 
-/* Header bar */
+/* Page container */
+#page-container {
+  bottom: 0;
+  overflow-y: hidden;
+  position: absolute;
+  top: 0;
+}
 
-header {
-  align-items: center;
-  background-color: rgb(33, 150, 243);
-  display: flex;
-  flex-direction: row;
-  font-size: 20pt;
-  height: 56px;
-  justify-content: flex-start;
-  padding: 0 16px;
+#page-container > * {
+  height: 100%;
+}
+
+@media screen and (min-width: 601px) {
+  #page-container {
+    left: 155px;
+    width: calc(100% - 155px);
+  }
 }
 
-.title {
-  color: white;
-  display: inline-block;
-  margin-left: 8px;
+@media screen and (max-width: 600px) {
+  #page-container {
+    width: 100%;
+  }
 }

[ortuno, 2016/12/05 05:45:28]

What about:

/* Page container */
#page-container {
  margin-left: 155px;
}

@media screen and (max-width: 600px) {
  #page-container {
    margin-left: 0;
  }
}

/* Page content */
.page-content {
  padding: 24px 16px;
}

Does that miss anything?

[mbrunson, 2016/12/06 00:25:45]

Done.

 
+/* Page header */
+#page-container header {

[ortuno, 2016/12/05 05:45:28]

I think we can simplify things a bit if we used flex and an image for the
button:

/* Page header */
#page-container header {
  background-image: linear-gradient(
      white, white 40%, rgba(255, 255, 255, 0.92));
  border-bottom: 1px solid #eee;
  display: flex;
  align-items: center;
  padding-top: 8px;
}

#page-container header > h1 {
  /* TODO: Figure out correct left margin */
  margin: 13px 0 13px 12px;
}

.menu-btn {
  background-color: white;
  /* TODO: figure out image */
  background-image: url(../options/info.svg);
  background-size: cover;
  border: 0;
  display: none;
  height: 24px;
  /* TODO: Figure out correct margin */
  margin-left: 12px;
  width: 24px;
}

@media screen and (max-width: 600px) {
  .menu-btn {
    display: block;
  }
}

[mbrunson, 2016/12/06 00:25:45]

Done.

+  background-image: linear-gradient(
+      white, white 40%, rgba(255, 255, 255, 0.92));
+  border-bottom: 1px solid #eee;
+}
 
-/* Device table */
+#page-container header > h1 {
+    margin: 0;
+    padding: 21px 0 13px;
+}
+
+.menu-btn {
+  background: none;
+  border: 0;
+  height: 24px;
+  left: 18px;
+  padding: 0;
+  position: absolute;
+  top: 18px;
+  width: 24px;
+}
+
+.menu-btn::before {
+  background: rgb(92, 97, 102);
+  box-shadow: 0 0.5em 0 0 rgb(92, 97, 102), 0 1em 0 0 rgb(92, 97, 102);
+  content: '';
+  height: 3px;
+  left: 0;
+  position: absolute;
+  top: 3px;
+  width: 24px;
+}
+
+@media screen and (min-width: 601px) {
+  .menu-btn {
+    display: none;
+  }
+}
+
+@media screen and (max-width: 600px) {
+  #page-container header {
+    left: 0;
+  }
+
+  #page-container header > h1 {
+    padding-left: 64px;
+  }
+}
+
+/* Page content */
+.page-content {
+  max-height: calc(100% - 102px);
+  overflow-y: auto;
+  padding: 24px 16px;
+}
+

[ortuno, 2016/12/05 05:45:28]

If we live #sidebar without "position: absolute" then all of its children will
be positioned relative to the nearest ancestor who has position: absolute or
position: relative in this case body. Since we don't want that please a rule for
#sidebar:

#sidebar {
  bottom: 0;
  left: 0;
  position: fixed;
  right: 0;
  top: 0;
  width: 155px;
  transition: visibility 0.2s ease;
}

@media screen and (max-width: 600px) {
  #sidebar {
    width: auto;
    visibility: hidden;
  }

  #sidebar.open {
    visibility: visible;
  }
}

[mbrunson, 2016/12/06 00:25:45]

Done.

+/* Sidebar */
+.sidebar-content {
+  background-color: white;
+  bottom: 0;
+  position: absolute;
+  top: 0;
+  width: 155px;
+}
+
+.sidebar-content ul {
+  list-style-type: none;
+  padding: 0;
+}
+
+.sidebar-content button {
+    background-color: transparent;
+    border: 0;
+    color: #999;
+    cursor: pointer;
+    font: inherit;
+    line-height: 1.417em;
+    margin: 6px 0;
+    padding: 0;
+}
+
+.sidebar-content li {
+  -webkit-border-start: 6px solid transparent;
+  -webkit-padding-start: 18px;
+  cursor: pointer;
+  margin: 6px 0;
+}
+
+.sidebar-content .selected {
+  -webkit-border-start-color: rgb(78, 87, 100);
+}
 
+.sidebar-content .selected > button {
+  color: rgb(70, 78, 90);
+}
+
+.sidebar-content li:hover {
+  background-color: #E0E0E0;
+}
+
+.overlay {
+  background-color: black;
+  bottom: 0;
+  display: none;

[ortuno, 2016/12/05 05:45:27]

We are already changing the opacity so might as well use that to show/hide the
sidebar:

.overlay {
  background-color: rgba(0, 0, 0, .5);
  bottom: 0;
  left: 0;
  opacity: 0;
  position: absolute;
  right: 0;
  top: 0;
  transition: opacity 225ms;
}

@media screen and (max-width: 600px) {
  .open .overlay {
    opacity: 1;
  }
}

[mbrunson, 2016/12/06 00:25:45]

Opacity only changes the transparency. Since the overlay is on top of the entire
page, it steals clicks from the main content. I set it to display:none so it
doesn't do this when it's invisible.

It would probably be better to use the visibility property and animate that.
Done.

[ortuno, 2016/12/06 10:16:04]

Right, but now we set visibility on #sidebar, so I think we can still use
opacity here.

[mbrunson, 2016/12/07 01:12:13]

Yes. But the sidebar is visible by default on large screens so it covers the
sidebar content and prevents it from capturing mouse events since it's
absolutely positioned.

[ortuno, 2016/12/07 02:58:17]

Ah right. On my version I had sidebar-content as position absolute as well. Note
that visibility doesn't have intermediate steps because it's discrete. It's
either hidden or visible. If you look at the animation you can see that it
doesn't fade out, it just disappears after a bit.

[mbrunson, 2016/12/07 04:26:07]

Ah I see. I added opacity as well and just set the visibility transition
duration such that it's hidden when opacity hits 0. Done.

+  left: 0;
+  opacity: 0;
+  position: fixed;

[ortuno, 2016/12/05 05:45:28]

If you add #sidebar { position: fixed; } then this can just be absolute.

[mbrunson, 2016/12/06 00:25:45]

Done.

+  right: 0;
+  top: 0;
+  transition: opacity 225ms;
+  transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
+}
+
+@media screen and (min-width: 601px) {

[ortuno, 2016/12/05 05:45:28]

I'm curious as to why you use the following pattern:

@media screen and (min-width: x + 1) {
}

@media screen and (max-width: x) {
}

?

It makes it really easy to loose track of the values since you now have to keep
track of 3 rule sets(In this case you are setting the cursor twice). I think
keeping just two sets of rules (base and max-width) should be enough.

[mbrunson, 2016/12/06 00:25:45]

Yeah. I originally wanted to make things explicit between what's shared, large
screen only, and small screen only. But that pattern doesn't make much sense
here since I'm only addressing two configurations. I'll consolidate things here.
Done.

+  .sidebar-content li {
+    cursor: pointer;
+    line-height: 30px;
+  }
+
+  .sidebar-content .selected {
+    cursor: default;

[ortuno, 2016/12/05 05:45:27]

Why change this depending on size?

[mbrunson, 2016/12/06 00:25:44]

I don't think I need this anymore actually.

+    pointer-events: none;
+  }
+}
+
+@media screen and (max-width: 600px) {
+  .sidebar-content {
+    left: -155px;

[ortuno, 2016/12/05 05:45:28]

Prefer to use transform when animating for two reasons:

1. transform operations are optimized by the compositor
2. top, right, bottom, left could affect layout so css has to recalculate the
whole layout.

[mbrunson, 2016/12/06 00:25:45]

Done.

+    transition: left 195ms;
+    transition-timing-function: cubic-bezier(0.4, 0, 0.6, 1);
+  }
+
+  .sidebar-content li {
+    line-height: 48px;

[ortuno, 2016/12/05 05:45:27]

Why do you change this in smaller screens? It looks really jumpy when you go
from 600px -> 601px.

[mbrunson, 2016/12/06 00:25:45]

The area for the buttons should be bigger to compensate for the comparatively
smaller size of the buttons on devices with smaller screens. This is the
recommended value for the size of touch targets on mobile devices.

https://material.google.com/layout/metrics-keylines.html#metrics-keylines-tou...

Keeping the 48px size makes the sidebar buttons very large on desktop monitors,
so I changed it just for smaller screens. Ideally, there would be a third type
of layout for tablets and mobile devices in landscape to minimize the changes
between layouts, but I will save this for a later patch if it becomes an issue.

[ortuno, 2016/12/06 10:16:04]

I see. What about 40px? Seems like an acceptable size based on densed lists in
[1] and doesn't look too big on bigger screenss

[1] https://material.google.com/components/lists.html#lists-specs

[mbrunson, 2016/12/07 01:12:13]

Done.

+  }
+
+  .sidebar-content.open {

[ortuno, 2016/12/05 05:45:28]

We can save ourselves some js lines by adding the open class to 'aside'
directly. Then our css rules would look like the following:

.open .sidebar-content {
 /* ... */
}

.open .overlay {
 /* ... */
}

[mbrunson, 2016/12/06 00:25:45]

Done.

+    left: 0;
+    transition: left 225ms;
+    transition-timing-function: cubic-bezier(0.4, 0, 0.2, 1);
+  }
+
+  .overlay.open {
+    display: block;
+    opacity: 0.5;
+  }
+}
+
+/* Device table */
 table {
-  border: 1px solid #ccc;
   border-collapse: collapse;
   margin: 0;
   padding: 0;
   width: 100%;
 }
 
-table tr {
-  border: 1px solid #ddd;
-  padding: 5px;
-}
-
 table th,
 table td {
-  padding: 10px;
-  text-align: center;
+  border: 1px solid rgb(217, 217, 217);
+  padding: 7px;
 }
 
 table th {
-  font-size: 14px;
-  letter-spacing: 1px;
-  text-transform: uppercase;
+  background-color: rgb(240, 240, 240);
+  font-weight: normal;
+}
+
+table .removed {
+  background-color: #BDBDBD;
 }
 
 @media screen and (max-width: 600px) {
-  table {
-    border: 0;
-  }
   table thead {
     display: none;
   }
-  table tr {
-    border-bottom: 2px solid #ddd;
-    display: block;
-  }
+
   table td {
-    border-bottom: 1px dotted #ccc;
     display: block;
-    font-size: 13px;
     text-align: right;
   }
-  table td:last-child {
-    border-bottom: 0;
-  }
+
   table td::before {
     content: attr(data-label);
     float: left;
     font-weight: bold;
-    text-transform: uppercase;
   }
 }
-
-/* Device Row */
-table .removed {
-  background-color: #BDBDBD;
-}