Skip to content
This repository was archived by the owner on Jul 30, 2019. It is now read-only.

Adjust Spacing on Auction Details Page - #237

Merged
adelevie merged 1 commit into
18F:developfrom
cwarden:216-auction-details-spacing
Jan 13, 2016
Merged

Adjust Spacing on Auction Details Page#237
adelevie merged 1 commit into
18F:developfrom
cwarden:216-auction-details-spacing

Conversation

@cwarden

@cwarden cwarden commented Jan 13, 2016

Copy link
Copy Markdown
Contributor

Reduce vertical spacing in description on auction details page.

Fixes #216.

How does this look?

2016-01-13-114251_936x1245_scrot

Reduce vertical spacing in description on auction details page.

Fixes 18F#216.
@vdavez

vdavez commented Jan 13, 2016

Copy link
Copy Markdown
Contributor

LGTM! 🎉 Thanks @cwarden!

@cwarden

cwarden commented Jan 13, 2016

Copy link
Copy Markdown
Contributor Author

Great, @vzvenyach. The requirements on this one were light. Let me know if you have other markdown you want me to test.

adelevie added a commit that referenced this pull request Jan 13, 2016
Adjust Spacing on Auction Details Page
@adelevie
adelevie merged commit 0a1b54f into 18F:develop Jan 13, 2016
@mikeendale

Copy link
Copy Markdown

Saw a tweet. 👍 for the quick turn around. If you're interested, here is a slightly refactored of the pull request.

h1, h2, h3, h4, h5, h6 {
  line-height: 1.1em;
  margin-top: 0.2em;
  margin-bottom: 0.1em;
}
h1 {
    margin-bottom: 0.5em;
}
p, li {
  line-height: 1.3em;
}
ul {
  margin-bottom: 0.5em;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@vzvenyach @cwarden why do we need all this added code? All that's required is removing:

.github-issue {
   white-space: pre-line;
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I found when just removing white-space: pre-line, the vertical spacing was still a bit larger than the 30px max desired.

2016-01-14-074916_1029x1313_scrot

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The requirement was that it was "significantly reduced." The 30px was stated as "e.g." which means for example, not the requirement. The requirement was to remove the erroneous vertical spacing which was created by white-space: pre-line;. The Draft U.S. Web Design Standards defines the vertical white space and was created for optimal legibility.

@maya maya mentioned this pull request Jan 14, 2016
@cwarden

cwarden commented Jan 18, 2016

Copy link
Copy Markdown
Contributor Author

@maya, your comments suggest some good questions about what tasks are best suited for micro-purchasing, and what level of detail in the requirements is optimal. I've written up some thoughts on my blog.

@vdavez

vdavez commented Jan 18, 2016

Copy link
Copy Markdown
Contributor

@cwarden Thanks! Looking forward to reading it!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants