chore(pfInfoStatusCard): Tweak infoStatusCard as per UX guidance #632

AllenBW · 2017-09-22T13:55:28Z

Description

a continuation of #630
@serenamarie125 @jennyhaines

Show me the 🤑

dtaylor113 · 2017-09-22T14:27:31Z

src/card/info-status/info-status-card.html

-       ng-repeat="item in $ctrl.status.info track by $index"></p>
-    <p ng-if="!$ctrl.shouldShowHtmlContent" ng-bind="item"
-       ng-repeat="item in $ctrl.status.info track by $index"></p>
+    <div ng-if="$ctrl.shouldShowHtmlContent" class="info-content" ng-bind-html="$ctrl.trustAsHtml(item)"


nit. but I think 'showHtmlContent' would be enough of a description :-)

@dtaylor113 I like your nits. Name follows other names around though, looking for buyin to rid ourselves of should in the whole of this work? pfinfostatuscard

Probably not in this PR if it's being used elsewhere. -thanks

AllenBW · 2017-09-22T14:46:40Z

As per convo with @jennyhaines workin on that egregious padding when including a title

cdcabrera

Minor formatting changes. I somehow missed the missing semicolons in the spec file earlier.

A few semicolons seem to be missing from this file, I probably missed a few too...
... 2, 7, 9, 14, 17, 28, 42, 54, 58, 74, 76, 80, 83, 86, 89,92, 95, 97, 109, 111, 114, 115, 119

cdcabrera · 2017-09-22T15:56:08Z

test/card/info-status/info-status-card.component.spec.js

@@ -73,7 +73,7 @@ describe('Component: pfInfoStatusCard', function () {

      element = compileCard('<pf-info-status-card status="status"></pf-info-status-card>', $scope)


Missing semicolon, looks like in quite a few spots within the file...

bahahahah eeeekkkk other project I'm on uses standard.js too funny, rubymine doesn't seem to respect that 🤦‍♂️

cdcabrera · 2017-09-22T15:56:40Z

src/card/info-status/info-status-card.html

@@ -5,18 +5,18 @@
         class="info-img"/>
    <span class="info-icon {{$ctrl.status.iconClass}}"></span>
  </div>
-  <div>
+  <div class="info-content-container">
    <h2 class="card-pf-title" ng-if="$ctrl.status.title">
      <a href="{{$ctrl.status.href}}" ng-if="$ctrl.status.href">
        <span class="">{{$ctrl.status.title}}</span>
      </a>
      <span ng-if="!$ctrl.status.href">
            <span class="">{{$ctrl.status.title}}</span>


Minor indent

what where?

looks like you got it

jeff-phillips-18 · 2017-09-22T17:18:02Z

src/card/card.less

    .info-icon {
      font-size: 50px;
    }
    .info-img {
      max-height: 50px;
    }
  }
+  .info-content-container {


We need to name this such that it is a PF thing. How about .card-pf-info-content? not sure we need to add -container

down with that name!
wil change the img one accordingly

jeff-phillips-18 · 2017-09-22T17:23:49Z

src/card/info-status/info-status-card.html

-       ng-repeat="item in $ctrl.status.info track by $index"></p>
-    <p ng-if="!$ctrl.shouldShowHtmlContent" ng-bind="item"
-       ng-repeat="item in $ctrl.status.info track by $index"></p>
+    <div ng-if="$ctrl.shouldShowHtmlContent" class="info-content" ng-bind-html="$ctrl.trustAsHtml(item)"


Where is ".info-content" defined? 👀

YOU NOTICED! its not, used for testing, also just in case user wants to grab each of those rows and do something with them, like a connivence class! 👍

can pull it and rework test if desired

You can leave it, might be nice for apps to use to add settings, how about a rename though? like card-pf-info-item?

yeah that's what i was thinking! will rename

jennyhaines · 2017-09-22T22:06:05Z

@AllenBW - I'm not sure how to review the visuals for the latest code changes to the cards. Could you post screenshots? Thank you!

AllenBW · 2017-09-22T22:36:45Z

@jennyhaines first post has latest screen shot

AllenBW · 2017-09-25T11:41:19Z

@jennyhaines If you compare the ss at the top of this pr to the #630 link in the first post, should make review a breeze!

jennyhaines · 2017-09-25T13:45:46Z

@AllenBW - The top spacing looks good! Thanks!! Only thing I'd suggest to change at this point is the spacing between the card title and the details. (for example, between "TinyCore-local" and "VM Name: aapde...", or "Favorite things" and the first icon). I'd decrease that spacing by 5px, and then I think the cards will be good to go!

AllenBW · 2017-09-25T14:04:33Z

@jennyhaines Updated! check ss in first post of pr 👍

jennyhaines

Looks good to me- Thanks!

dtaylor113 reviewed Sep 22, 2017

View reviewed changes

dtaylor113 approved these changes Sep 22, 2017

View reviewed changes

AllenBW force-pushed the chore/address-pfinfostatuscard-style-feedback branch from 1aea727 to 739a590 Compare September 22, 2017 15:16

cdcabrera requested changes Sep 22, 2017

View reviewed changes

AllenBW force-pushed the chore/address-pfinfostatuscard-style-feedback branch from 739a590 to 705436d Compare September 22, 2017 16:08

cdcabrera approved these changes Sep 22, 2017

View reviewed changes

cdcabrera requested a review from jeff-phillips-18 September 22, 2017 16:38

jeff-phillips-18 requested changes Sep 22, 2017

View reviewed changes

AllenBW force-pushed the chore/address-pfinfostatuscard-style-feedback branch 2 times, most recently from 12bd3d0 to a31d5e8 Compare September 22, 2017 18:19

jeff-phillips-18 requested a review from jennyhaines September 22, 2017 18:41

AllenBW force-pushed the chore/address-pfinfostatuscard-style-feedback branch from a31d5e8 to abb4d29 Compare September 25, 2017 14:03

jennyhaines approved these changes Sep 25, 2017

View reviewed changes

AllenBW mentioned this pull request Sep 25, 2017

Resource Details Page - ie: bulk cards, everything is a card ManageIQ/manageiq-ui-service#910

Merged

10 tasks

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

View reviewed changes

AllenBW force-pushed the chore/address-pfinfostatuscard-style-feedback branch from abb4d29 to b6736c6 Compare September 25, 2017 17:27

fix(pfInfoStatusCard): Tweak infoStatusCard as per UX guidance

1f49434

AllenBW force-pushed the chore/address-pfinfostatuscard-style-feedback branch from b6736c6 to 1f49434 Compare September 25, 2017 17:28

cdcabrera merged commit 179d157 into patternfly:master Sep 26, 2017

AllenBW deleted the chore/address-pfinfostatuscard-style-feedback branch July 23, 2018 15:14

		@@ -73,7 +73,7 @@ describe('Component: pfInfoStatusCard', function () {

		element = compileCard('<pf-info-status-card status="status"></pf-info-status-card>', $scope)

chore(pfInfoStatusCard): Tweak infoStatusCard as per UX guidance #632

chore(pfInfoStatusCard): Tweak infoStatusCard as per UX guidance #632

Uh oh!

Conversation

AllenBW commented Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Show me the 🤑

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!

AllenBW commented Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cdcabrera left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

Choose a reason for hiding this comment

Uh oh!

AllenBW Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AllenBW Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jennyhaines commented Sep 22, 2017

Uh oh!

AllenBW commented Sep 22, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AllenBW commented Sep 25, 2017

Uh oh!

jennyhaines commented Sep 25, 2017

Uh oh!

AllenBW commented Sep 25, 2017

Uh oh!

jennyhaines left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AllenBW commented Sep 22, 2017 •

edited

Loading

AllenBW commented Sep 22, 2017 •

edited

Loading

cdcabrera left a comment •

edited

Loading

AllenBW Sep 22, 2017 •

edited

Loading

AllenBW Sep 22, 2017 •

edited

Loading

AllenBW commented Sep 22, 2017 •

edited

Loading