Skip to content

fix(pfTrendsChart): In compact layout, tightened up space #591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 25, 2017

Conversation

dtaylor113
Copy link
Member

Description

In compact layout, tightened space up between labels and chart:

PR Checklist

  • Unit tests are included
  • Screenshots are attached (if there are visual changes in the UI)
  • A Designer (@beanh66) is assigned as a reviewer (if there are visual changes in the UI)
  • A CSS rep (@cshinn) is assigned as a reviewer (if there are visual changes in the UI)

Previous:
screen shot 2017-08-17 at 4 27 49 pm

Now:
image

Fixes #579

@serenamarie125

@dtaylor113 dtaylor113 requested review from cshinn and beanh66 August 23, 2017 16:52
@beanh66
Copy link
Member

beanh66 commented Aug 23, 2017

@dtaylor113 is there a rawgit link I should checkout?

@@ -19,7 +19,7 @@
</div>
<div ng-switch-when="compact" class="trend-card-compact-pf">
<div class="row trend-row">
<div class="col-sm-4 col-md-4">
<div class="col-sm-2 col-md-2">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now would be a good time to lose the useless col-md values on these columns since they are the same as the col-sm value. Would clean it up and one less selector in the less file above.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, fixed

@dtaylor113
Copy link
Member Author

@dtaylor113 is there a rawgit link I should checkout?

Hi @beanh66, here is the ngDoc example

@@ -19,7 +19,7 @@
</div>
<div ng-switch-when="compact" class="trend-card-compact-pf">
<div class="row trend-row">
<div class="col-sm-4 col-md-4">
<div class="col-md-2">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you wanted col-sm-2 here so that at small resolutions these stay on a single line. Only at xs should they be vertical. This goes for all the changes here.

.col-sm-10, .col-md-10 {
padding-left: 0;
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should only need the col-sm- selectors now.

@dtaylor113
Copy link
Member Author

Hi @jeff-phillips-18, I (hopefully) fixed the col selectors

@beanh66
Copy link
Member

beanh66 commented Aug 25, 2017

@dtaylor113 The spacing looks good! I'm curious in general, why does the label get stacked on top instead of staying on the right or left at a view like this:
screen shot 2017-08-25 at 10 52 59 am

@beanh66
Copy link
Member

beanh66 commented Aug 25, 2017

@dtaylor113 It seems like it should only stack at an xs resolution. @jeff-phillips-18 thought this is how it currently works, but maybe the ngDocs just aren't updated with the latest code?

@jeff-phillips-18
Copy link
Member

@beanh66 The rawgit branch hasn't been updated. I pulled and tested and the latest changes correctly stack only at the xs resolution.

@jeff-phillips-18 jeff-phillips-18 merged commit c7c7f29 into patternfly:master Aug 25, 2017
@dtaylor113 dtaylor113 deleted the trend-chart branch October 19, 2017 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants