Skip to content
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

Purchases: Add site slug to SectionHeaders #2359

Merged
merged 3 commits into from
Jan 14, 2016

Conversation

breezyskies
Copy link
Contributor

This adds the site slug (primary domain) to the SectionHeaders for each site in /purchases, as suggested by @mikeshelton1503 in #170 (comment), to help make it easier to identify each site in the list.

Before After
screen shot 2016-01-12 at 3 20 55 pm screen shot 2016-01-12 at 3 31 21 pm

@mikeshelton1503 Note that in your original mockup, the SectionHeader label and slug were different colors, creating more hierarchy in the information presented here. I updated the color on the slug to add some contrast.

@breezyskies breezyskies added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc. labels Jan 12, 2016
@breezyskies breezyskies self-assigned this Jan 12, 2016
.purchases-site__slug {
color: darken( $gray, 15% );
font-size: 12px;
line-height: 28px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than set a large line-height here, presumably to control the vertical alignment, you can control the vertical alignment by adding align-items: baseline to .section-header.card(though you would need to qualify it with .purchases-site).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

@mikeshelton1503
Copy link
Contributor

Looks good, I just noted a couple minor CSS things. Also:

On smaller screens when there is a longer site name and site url the site name wraps, which is fine, however the line-height is too large:

image

@mikeshelton1503
Copy link
Contributor

Oh, one other note: It hadn't occurred to me until I saw it in action but should the site url be linked? My guts says no, however I can see users trying to click it. We could have it open the site in a new tab as to not pull users away from Purchases. I'm not sure though. @breezyskies what do you think?

@breezyskies breezyskies force-pushed the update/purchases-site-header branch from 093bcae to 927584b Compare January 14, 2016 15:25
@breezyskies
Copy link
Contributor Author

On smaller screens when there is a longer site name and site url the site name wraps, which is fine, however the line-height is too large:

Fixed so the name fades out instead of wrapping.

It hadn't occurred to me until I saw it in action but should the site url be linked?

I don't think there's a lot of value in the linking other than the potential to fix a usability issue when users try to click. We do present slugs / domains in other areas of Calypso that do not click through, but this is a unique case since it is kind of on it's own in the design, and a different color. I could go either way on this one.

@mikeshelton1503
Copy link
Contributor

👍 LGTM

@mikeshelton1503 mikeshelton1503 added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Jan 14, 2016
breezyskies added a commit that referenced this pull request Jan 14, 2016
Purchases: Add site slug to SectionHeaders
@breezyskies breezyskies merged commit c251cd4 into master Jan 14, 2016
@breezyskies breezyskies deleted the update/purchases-site-header branch January 14, 2016 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Purchase Management Related to managing purchases such as subscriptions, plans, history, auto-renew, cancellation, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants