Incorporation of UI animation effects for deployment transitions on the overview #1402

sg00dwin · 2017-04-06T21:30:13Z

The .row-expanded-top contains the pod template and animation block. .overview-animation-block contains metrics if active, and the overview-deployment-donut. During deployment the previous and current deployment donut charts are grouped and either slideIn from the left or slideDown at mobile widths. At >992 because of space constraints the pod-template and metrics fade out and hidden to allow for both donuts and arrow, and fade in when complete. As the previous-donut scales down and removed it fades out.

Markup structure is controlled by flexbox and 50% widths
Metrics are only beneath a tab when >480
Networking and builds sections using col-md-6 to match 50% widths above

Issues
[] at the start of animation, scale arrows and directional arrow display briefly before the row.previous donut chart is displayed.
[] at the end of the deployment, the pod-template and metrics nodes appear before the previous deployment donut is removed causing some position jumping.

Screen recording with source code of both issues
metrics
pod-tempate

sg00dwin · 2017-04-06T21:33:26Z

@jwforres @spadgett @rhamilto

rhamilto · 2017-04-07T12:49:05Z

Nice work, @sg00dwin!

spadgett · 2017-04-10T17:15:36Z

This change centers the donut when it was previously right-aligned. Is that intentional?

Before:

After:

spadgett

A few initial comments. I'd like @rhamilto help looking at the CSS

spadgett · 2017-04-10T16:54:34Z

app/styles/_overview.less

    }
  }
  .expanded-section {
    margin-top: 20px;
    > .row > [class^=col-] {
      margin-bottom: 10px;
+      @media (min-width: @screen-lg-min) {
+        // aligns columns with metrics
+        &:nth-child(2) {


I'd really like to avoid nth-child and instead give a class to the item that needs it.

spadgett · 2017-04-10T16:55:17Z

app/styles/_overview.less

@@ -284,6 +304,12 @@
    .nav-tabs > li.active > a .build-count {
      color: #4d5258;
    }
+    @media (min-width: @screen-xs-min) {
+      // metrics are displayed next to donut so remove tab


I'm confused: can we just remove the metrics tab from the markup?

spadgett · 2017-04-10T17:17:16Z

app/styles/_overview.less

@@ -346,7 +372,7 @@
    margin: 0 20px 10px;
  }
  overview-list-row + overview-list-row .list-pf-item.active {
-    // avoid double borders when list-pf-item is open
+  // avoid double borders when list-pf-item is open


Indentation

spadgett · 2017-04-10T17:20:05Z

app/views/overview/_list-row-expanded.html

+      }">
+        <div ng-if="row.state.breakpoint !== 'xs'" class="overview-pod-template" ng-class="{
+          'ng-enter': row.previous,
+          'hidden-sm hidden-md': row.previous && row.state.breakpoint >= 'md'


>= 'md' won't work since they're strings. You need to test each breakpoint individually.

I changed this to be !== 'lg', but the other way looked to be working for me. :)

spadgett · 2017-04-10T17:20:21Z

app/views/overview/_list-row-expanded.html

+
+        <div class="overview-deployment-donut" ng-class="{
+            'ng-enter': row.previous,
+            'stacked-template': row.state.breakpoint >= 'md'


>= 'md' won't work

sg00dwin · 2017-04-11T20:29:34Z

PR updated

sg00dwin · 2017-04-13T18:31:25Z

[test]

sg00dwin · 2017-04-13T18:32:36Z

@spadgett pr rebased. ptal

rhamilto · 2017-04-17T13:26:35Z

app/styles/_overview.less

+        }
+      }
+    }
+    .overview-animation-block {


Nit: I would love to see rules alphabetized by selector.

rhamilto · 2017-04-17T13:28:52Z

app/styles/_overview.less

@@ -225,6 +249,7 @@
    }
    .list-pf-container {
      display: block; // so that .word-break() will work in IE
+      overflow: hidden; // so animation offsets are hidden


Results in the donut tooltip being clipped. :(

spadgett · 2017-04-17T16:57:13Z

Test failure is a flake

Running "add-redirect-uri" task
oc --server https://localhost:8443 patch oauthclient/openshift-web-console -p {"redirectURIs":["https://localhost:9000/"]}
>> Error: The connection to the server localhost:8443 was refused - did you specify the right host or port?

…verview. The row-expanded-top contains the overview-pod-template and animation-block, which will contain metrics if active, and the overview-deployment-donut. During deployment the previous and current deployment donut charts are grouped and either slideIn from the left or slideDown at mobile widths. At >992 because of space constraints the pod-template and metrics fade out and hidden to allow for both donuts and arrow, and fade in when complete. As the previous-donut scales down and removed it fades out. - Markup structure is controlled by flexbox and 50% widths - Metrics are only beneath a tab when >480 - Networking and builds sections using col-md-6 to match 50% widths above

sg00dwin · 2017-04-17T18:50:39Z

@rhamilto @spadgett updated pr

openshift-bot · 2017-04-17T18:55:08Z

Evaluated for origin web console test up to fad6740

openshift-bot · 2017-04-17T19:11:25Z

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1237/) (Base Commit: 9143df0)

spadgett · 2017-04-18T15:48:24Z

[merge]

openshift-bot · 2017-04-18T15:55:09Z

Evaluated for origin web console merge up to fad6740

openshift-bot · 2017-04-18T15:55:18Z

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1242/) (Base Commit: b5de937)

spadgett reviewed Apr 10, 2017

View reviewed changes

spadgett requested a review from rhamilto April 10, 2017 17:22

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 10, 2017

sg00dwin force-pushed the animation-overview branch 4 times, most recently from 848f42e to 1c8454f Compare April 11, 2017 20:28

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 11, 2017

sg00dwin force-pushed the animation-overview branch from 1c8454f to c195d37 Compare April 12, 2017 14:56

openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 12, 2017

sg00dwin force-pushed the animation-overview branch from c195d37 to d8c6a7b Compare April 13, 2017 18:10

openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Apr 13, 2017

rhamilto suggested changes Apr 17, 2017

View reviewed changes

sg00dwin force-pushed the animation-overview branch from d8c6a7b to 60ba41a Compare April 17, 2017 16:02

openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 17, 2017

sg00dwin force-pushed the animation-overview branch 2 times, most recently from 6e257ad to 45bb9d1 Compare April 17, 2017 18:24

sg00dwin force-pushed the animation-overview branch from 45bb9d1 to fad6740 Compare April 17, 2017 18:48

rhamilto approved these changes Apr 17, 2017

View reviewed changes

spadgett approved these changes Apr 18, 2017

View reviewed changes

openshift-bot merged commit 82ed883 into openshift:master Apr 18, 2017

sg00dwin deleted the animation-overview branch April 18, 2017 16:30

Incorporation of UI animation effects for deployment transitions on the overview #1402

Incorporation of UI animation effects for deployment transitions on the overview #1402

Uh oh!

Conversation

sg00dwin commented Apr 6, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sg00dwin commented Apr 6, 2017

Uh oh!

rhamilto commented Apr 7, 2017

Uh oh!

spadgett commented Apr 10, 2017

Uh oh!

spadgett left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sg00dwin commented Apr 11, 2017

Uh oh!

sg00dwin commented Apr 13, 2017

Uh oh!

sg00dwin commented Apr 13, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

spadgett commented Apr 17, 2017

Uh oh!

sg00dwin commented Apr 17, 2017

Uh oh!

openshift-bot commented Apr 17, 2017

Uh oh!

openshift-bot commented Apr 17, 2017

Uh oh!

spadgett commented Apr 18, 2017

Uh oh!

openshift-bot commented Apr 18, 2017

Uh oh!

openshift-bot commented Apr 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

sg00dwin commented Apr 6, 2017 •

edited

Loading

openshift-bot commented Apr 18, 2017 •

edited

Loading