Notification panel transitions and visual enhancements #1997

sg00dwin · 2017-09-01T17:28:18Z

Update to PatternFly 3.27.1

Fixes #1993

sg00dwin · 2017-09-01T17:37:26Z

@beanh66 @serenamarie125
@spadgett @rhamilto

At expanded view, long notification messages can grow quite tall. I explored stacking the message beneath the title to give it space to expand. And improve readability. Any objections?

rhamilto · 2017-09-01T17:36:14Z

app/styles/_notifications.less

    // TODO: hack to eliminate side-to-side wobble
    // There is a hard-coded <h4> that wraps the header:
    //   - https://github.com/patternfly/angular-patternfly/issues/539
    // I'm putting quite a bit of markup inside the header,
    // need to tinker to eliminate this rule.
-    overflow: hidden;
+    overflow:hidden;
+    font-size: @font-size-h4;


rhamilto · 2017-09-01T17:40:22Z

app/styles/_navbar-vertical.less

@@ -87,6 +87,12 @@
      @media(max-width: @screen-xs-max) {
        padding: 10px 15px 10px 5px;
      }
+      .badge {


I had absolutely positioned .badge in one of my PRs. It looks like it got rebased away. :( Can we redo the absolute positioning? Otherwise the width of the nav item changes with the coming and going of the badge.

Nevermind. The changes are in master. But perhaps you should combine the two rules so they're not in two different files?

rhamilto · 2017-09-01T17:40:29Z

app/styles/_navbar-vertical.less

+        background-color: @color-pf-blue-300;
+        border: 2px solid @navbar-os-bg-color;
+        width: 12px;
+        height: 12px;


rhamilto · 2017-09-01T17:40:58Z

app/styles/_notifications.less

+      padding: 10px 10px 10px 42px;
+      .text-muted;
+    }
+    .drawer-pf-notification-info {


rhamilto · 2017-09-01T17:41:10Z

app/styles/_notifications.less

+        font-weight: normal;
+        .word-break();
+      }
+      .drawer-pf-notification-info {


serenamarie125 · 2017-09-01T17:51:34Z

@sg00dwin this looks much better IMO!

sg00dwin · 2017-09-01T18:13:19Z

changes updated

jeff-phillips-18 · 2017-09-01T18:15:05Z

app/styles/_notifications.less

@@ -31,6 +42,9 @@ notification-drawer-wrapper {
      }
    }
  }
+  .drawer-pf-close, .drawer-pf-toggle-expand {
+    padding: 2px 10px;
+  }


We should give feedback to patternfly on things like this. Shouldn't have to be overriding these settings.

@sg00dwin why are we doing this? Agree that we shouldn't be overriding things. If we need PF to change something let me know. If they cannot do it in time, then we can override with a plan to change later.

@jeff-phillips-18 @serenamarie125 Makes sense. For context, @sg00dwin added a few pixels of extra padding between the icon and the edge of the drawer.

@sg00dwin It might just be easier to remove this.

Before:

After:

Felt the click target as too small, 5px padding between icon and edge.
I'll revert and suggest issue upstream.

before

after

We should give feedback to patternfly on things like this. Shouldn't have to be overriding these settings.

While I don't disagree, sometimes it's infinitely easier just to fix a problem in the code that you control.

Totally agree that what you have done looks much better!

I do think that if we take those liberties, please call them out so that UX can review and we can make a plan get them into PatternFly for consistency :D Understand your point @rhamilto but when our customers are dealing with a portfolio of products, everyone doing their own things just causes further issues.

+1 prob agree to both...temp fix here then ensure an issue upstream to track/migrate it.

jeff-phillips-18 · 2017-09-01T18:20:10Z

app/styles/_navbar-vertical.less

+    .nav-item-iconic .badge {
+      background-color: @color-pf-blue-300;
+      border: 2px solid @navbar-os-bg-color;
+      height: 12px;


I don't think this should be straying from the Patternfly design, @openshift/team-ux-review ?

@sg00dwin What is the before and after on this change?

If @openshift/team-ux-review agrees it's better, we could look at contributing it upstream in Patternfly.

@sg00dwin again, why are we doing this? Was this suggested by @jhaines?

If there's a good reason, let's get this into PF. LOTS of products are using the notification drawer, we want this to be consistent if it's indeed something that improves the component.

discussed with rhamilto, done to improve identification of unread notification.
Will revert

before

after

@serenamarie125 Opinion? I think the badge stands out better with the black around it. Let us know, and we can add submit this in Patternfly.

Before:

After:

I think it's worth noting the inspiration for the border around the badge came from Github.

again, imo @sg00dwin @rhamilto I think your proposal looks great, and i did immediately recognize that it was consistent with GitHub.

If you all are willing to put up a PR in PatternFly, i think that would be terrific!

Update to PatternFly 3.27.1

benjaminapetersen

Much improved readability. Also like the fade in effect, and the positioning from top matches heading in both desktop & mobile. Now if my PR would just merge that would bump pf & get rid of the expand icon @ mobile. 🙂

spadgett

LGTM

@sg00dwin has removed the two items we talked about from this PR with plans to submit to Patternfly

spadgett · 2017-09-06T14:39:52Z

@jeff-phillips-18 @rhamilto Another look?

rhamilto · 2017-09-06T17:41:29Z

[merge]

spadgett · 2017-09-06T18:57:03Z

Flake #2016

[merge][severity: bug]

openshift-bot · 2017-09-06T19:08:47Z

Evaluated for origin web console merge up to 15e836f

openshift-bot · 2017-09-06T19:08:56Z

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/merge_pull_request_origin_web_console/126/) (Base Commit: 73c7420) (PR Branch Commit: 15e836f) (Extended Tests: bug)

spadgett requested a review from rhamilto September 1, 2017 17:34

rhamilto suggested changes Sep 1, 2017

View reviewed changes

spadgett requested a review from jeff-phillips-18 September 1, 2017 17:44

sg00dwin force-pushed the notifications branch from 90a8cf2 to 6ec9bb4 Compare September 1, 2017 18:11

jeff-phillips-18 reviewed Sep 1, 2017

View reviewed changes

sg00dwin force-pushed the notifications branch from 6ec9bb4 to 8aba15e Compare September 1, 2017 18:18

jeff-phillips-18 reviewed Sep 1, 2017

View reviewed changes

sg00dwin force-pushed the notifications branch from 8aba15e to d597530 Compare September 1, 2017 19:05

Notification panel transitions

15e836f

Update to PatternFly 3.27.1

sg00dwin force-pushed the notifications branch from d597530 to 15e836f Compare September 1, 2017 19:11

benjaminapetersen approved these changes Sep 1, 2017

View reviewed changes

benjaminapetersen mentioned this pull request Sep 1, 2017

updates notification drawer to support mobile patternfly/patternfly-3#717

Merged

2 tasks

spadgett approved these changes Sep 1, 2017

View reviewed changes

LHinson mentioned this pull request Sep 2, 2017

Notification Drawer: Expanded View for PatternFly patternfly/patternfly-design#405

Closed

2 tasks

jeff-phillips-18 approved these changes Sep 6, 2017

View reviewed changes

rhamilto approved these changes Sep 6, 2017

View reviewed changes

openshift-bot merged commit 38eb4eb into openshift:master Sep 6, 2017

sg00dwin deleted the notifications branch September 7, 2017 14:20

This was referenced Sep 12, 2017

Fix(notifications): enhancement: patternfly/patternfly-3#748

Merged

fix(drawer-pf-notifications): increase horizontal padding around drawer-pf-toggle-expand and drawer-pf-close controls patternfly/patternfly-3#749

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Notification panel transitions and visual enhancements #1997

Notification panel transitions and visual enhancements #1997

sg00dwin commented Sep 1, 2017

sg00dwin commented Sep 1, 2017

rhamilto Sep 1, 2017

rhamilto Sep 1, 2017 •

edited

Loading

rhamilto Sep 1, 2017 •

edited

Loading

rhamilto Sep 1, 2017

rhamilto Sep 1, 2017

rhamilto Sep 1, 2017

serenamarie125 commented Sep 1, 2017 •

edited

Loading

sg00dwin commented Sep 1, 2017

jeff-phillips-18 Sep 1, 2017

serenamarie125 Sep 1, 2017

spadgett Sep 1, 2017

sg00dwin Sep 1, 2017

rhamilto Sep 1, 2017

serenamarie125 Sep 1, 2017

benjaminapetersen Sep 1, 2017

jeff-phillips-18 Sep 1, 2017

spadgett Sep 1, 2017

serenamarie125 Sep 1, 2017

sg00dwin Sep 1, 2017

spadgett Sep 1, 2017 •

edited

Loading

rhamilto Sep 1, 2017

serenamarie125 Sep 1, 2017 •

edited

Loading

benjaminapetersen left a comment •

edited

Loading

spadgett left a comment

spadgett commented Sep 6, 2017

rhamilto commented Sep 6, 2017

spadgett commented Sep 6, 2017

openshift-bot commented Sep 6, 2017

openshift-bot commented Sep 6, 2017 •

edited

Loading

Notification panel transitions and visual enhancements #1997

Notification panel transitions and visual enhancements #1997

Conversation

sg00dwin commented Sep 1, 2017

sg00dwin commented Sep 1, 2017

Choose a reason for hiding this comment

rhamilto Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

rhamilto Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serenamarie125 commented Sep 1, 2017 • edited Loading

sg00dwin commented Sep 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

spadgett Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

serenamarie125 Sep 1, 2017 • edited Loading

Choose a reason for hiding this comment

benjaminapetersen left a comment • edited Loading

Choose a reason for hiding this comment

spadgett left a comment

Choose a reason for hiding this comment

spadgett commented Sep 6, 2017

rhamilto commented Sep 6, 2017

spadgett commented Sep 6, 2017

openshift-bot commented Sep 6, 2017

openshift-bot commented Sep 6, 2017 • edited Loading

rhamilto Sep 1, 2017 •

edited

Loading

rhamilto Sep 1, 2017 •

edited

Loading

serenamarie125 commented Sep 1, 2017 •

edited

Loading

spadgett Sep 1, 2017 •

edited

Loading

serenamarie125 Sep 1, 2017 •

edited

Loading

benjaminapetersen left a comment •

edited

Loading

openshift-bot commented Sep 6, 2017 •

edited

Loading