Fix analysis-view z-index

Fix analysis-view z-index.

Recent changes to the side-panel-container caused a regression in which its
tab-strip has a higher z-index than the analysis-view, so that when the
analysis-view is dragged up over the side-panel-container, a visual glitch
appears in which the tab-strip is visible over the analysis-view.

This CL elevates the z-index of the analysis-view above that of the
side-panel-container.

BUG=catapult:#2365

Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/baa2999e8462ec4c153d2c95961d5536ecc201fc

[benjhayden, 2016-05-25 16:05:42]

Description was changed from

==========
Fix analysis-view z-index
==========

to

==========
Fix analysis-view z-index.

Recent changes to the side-panel-container caused a regression in which its
tab-strip has a higher z-index than the analysis-view, so that when the
analysis-view is dragged up over the side-panel-container, a visual glitch
appears in which the tab-strip is visible over the analysis-view.

This CL elevates the z-index of the analysis-view above that of the
side-panel-container.

BUG=catapult:#2365
==========

[benjhayden, 2016-05-25 16:05:52]

benjhayden@chromium.org changed reviewers:
+ nednguyen@google.com

[nednguyen, 2016-05-25 16:07:26]

On 2016/05/25 16:05:52, benjhayden_chromium wrote:

Should this z-index be set by the container of "tr-ui-a-analysis-view" instead
to avoid this kind of bug in the future?

[nduca, 2016-05-25 16:30:27]

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

[nduca, 2016-05-25 16:30:28]

feels like we have a deeper bug here... why're we setting z index anywhere?

[benjhayden, 2016-05-25 17:46:59]

> Should this z-index be set by the container of "tr-ui-a-analysis-view" instead
to avoid this kind of bug in the future?

The analysis-view is contained directly by the timeline-view, which is the root
app-level element.
If you're suggesting that we switch to a paradigm in which z-index is only set
by containers (which are aware of their own z-index and the intended stacking
order of their children) onto their children elements, and never set by ignorant
children, then I'd be supportive of that paradigm, but it would be a much bigger
patch than this.


On 2016/05/25 at 16:30:28, nduca wrote:
> feels like we have a deeper bug here... why're we setting z index anywhere?

My understanding of the crbugs is that the drag-handles create an expectation of
a stacking order in the UI: mouse-mode-selector > analysis-view.drag-handle >
analysis-view > side-panel.drag-handle > side-panel > track heading >
timeline-track-view.

https://bugs.chromium.org/p/chromium/issues/detail?id=613099
https://bugs.chromium.org/p/chromium/issues/detail?id=614688

mouse-mode-selector and drag-handle already used z-index to accomplish their
part in the stacking order. This CL follows their lead.

This is followup work from Ned's request to fix the inflexible side-panel
widths:
https://codereview.chromium.org/1978673002

I can understand why you feel that there might be deeper bug here, but besides
the fact that z-index is effectively a global, do you see another particular
deeper bug here?

[benjhayden, 2016-05-25 17:47:10]

> Should this z-index be set by the container of "tr-ui-a-analysis-view" instead
to avoid this kind of bug in the future?

The analysis-view is contained directly by the timeline-view, which is the root
app-level element.
If you're suggesting that we switch to a paradigm in which z-index is only set
by containers (which are aware of their own z-index and the intended stacking
order of their children) onto their children elements, and never set by ignorant
children, then I'd be supportive of that paradigm, but it would be a much bigger
patch than this.


On 2016/05/25 at 16:30:28, nduca wrote:
> feels like we have a deeper bug here... why're we setting z index anywhere?

My understanding of the crbugs is that the drag-handles create an expectation of
a stacking order in the UI: mouse-mode-selector > analysis-view.drag-handle >
analysis-view > side-panel.drag-handle > side-panel > track heading >
timeline-track-view.

https://bugs.chromium.org/p/chromium/issues/detail?id=613099
https://bugs.chromium.org/p/chromium/issues/detail?id=614688

mouse-mode-selector and drag-handle already used z-index to accomplish their
part in the stacking order. This CL follows their lead.

This is followup work from Ned's request to fix the inflexible side-panel
widths:
https://codereview.chromium.org/1978673002

I can understand why you feel that there might be deeper bug here, but besides
the fact that z-index is effectively a global, do you see another particular
deeper bug here?

[nduca, 2016-05-26 16:43:42]

i don't quite get why drag-handle needs z index. that sounds like original sin.

[benjhayden, 2016-05-26 20:06:04]

Patch 2 unsets drag-handle's and side-panel-container's z-index, which leaves
this bug:
https://screenshot.googleplex.com/QhPWN5P8HC9.png
If you drag the side-panel-container's drag-handle far enough left, then it
looks like it's partially hidden by the track headings.
I outlined a few possible solutions in the original bug:
https://bugs.chromium.org/p/chromium/issues/detail?id=613099#c7

Happy to take suggestions. :-)

[benjhayden, 2016-05-26 23:30:39]

Thanks, Nat! PTAL

[nduca, 2016-05-26 23:31:39]

lgtm

[benjhayden, 2016-05-26 23:47:21]

The CQ bit was checked by benjhayden@chromium.org

[commit-bot: I haz the power, 2016-05-26 23:47:27]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2013843002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/2013843002/40001

[commit-bot: I haz the power, 2016-05-27 00:09:02]

Description was changed from

==========
Fix analysis-view z-index.

Recent changes to the side-panel-container caused a regression in which its
tab-strip has a higher z-index than the analysis-view, so that when the
analysis-view is dragged up over the side-panel-container, a visual glitch
appears in which the tab-strip is visible over the analysis-view.

This CL elevates the z-index of the analysis-view above that of the
side-panel-container.

BUG=catapult:#2365
==========

to

==========
Fix analysis-view z-index.

Recent changes to the side-panel-container caused a regression in which its
tab-strip has a higher z-index than the analysis-view, so that when the
analysis-view is dragged up over the side-panel-container, a visual glitch
appears in which the tab-strip is visible over the analysis-view.

This CL elevates the z-index of the analysis-view above that of the
side-panel-container.

BUG=catapult:#2365

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-05-27 00:09:03]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...