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

Increase header email column size, truncate email #1412

Merged
merged 2 commits into from
May 3, 2017

Conversation

el-mapache
Copy link
Contributor

@el-mapache el-mapache commented May 2, 2017

Why: Currently, longer emails can break into multiple lines in the
header.

How: Increased column size for welcome message+email. Truncate very
long emails. It was necessary to add a truncate wrapper class to the
parent element, since we want to selectively truncate only a portion of
the content, not the entire line.

Screenshots:

Shorter email:
screen shot 2017-05-02 at 12 07 55 pm

Longer email:
screen shot 2017-05-02 at 12 08 59 pm

**Why**: Currently, longer emails can break into multiple lines in the
header.

**How**: Increased column size for welcome message+email. Truncate very
long emails. It was necessary to add a truncate wrapper class to the
parent element, since we want to selectively truncate only a portion of
the content, not the entire line.
@el-mapache el-mapache requested a review from hursey013 May 2, 2017 16:07
@hursey013
Copy link
Member

hursey013 commented May 2, 2017

This looks good! I did notice a few related issues that we can either try and address in this PR or make separate issues for (let me know if thats what you want and I can create the issues).

I think we should apply the same truncation within the profile content. I think we may have lost that functionality during the last refresh:

screen shot 2017-05-02 at 12 50 14 pm

And also on mobile:
screen shot 2017-05-02 at 12 52 21 pm

@el-mapache
Copy link
Contributor Author

@hursey013 Oh yeah, that doesn't look good. I'll update the PR.

@el-mapache
Copy link
Contributor Author

el-mapache commented May 2, 2017

I added the truncate class to the header on mobile, and to each of the content rows in the personal information table!

screen shot 2017-05-02 at 1 07 25 pm

@el-mapache el-mapache force-pushed the ab-adjust-account-header-columns branch from c1809e5 to ef83ff9 Compare May 2, 2017 17:08
@hursey013
Copy link
Member

Header portion looks good, the email address in the profile information box seems to truncate too early on mobile and desktop though. Is it possible to expand the width of those columns or does that have unintended side effects?

screen shot 2017-05-02 at 1 29 39 pm

screen shot 2017-05-02 at 1 29 52 pm

@hursey013
Copy link
Member

On mobile at least, it seems like since the email address is on its own line it could probably span the width of the box.

@el-mapache
Copy link
Contributor Author

el-mapache commented May 2, 2017

It would just involve changing the column sizing there. I can't recall if there was a specific design reason they are sized the way they are, its probably fine to update them.

EDIT: I see now that we have the columns sized the way they are because the designs specified that each of the edit action buttons should fit inline. Most of them would with a smaller column size, but not the 'get a new key' button. If it seems fine to have that break onto a new line, I can update the columns.

@el-mapache
Copy link
Contributor Author

el-mapache commented May 2, 2017

The email won't ever be able to span the entire line however, its contained inside a parent div that needs to accommodate the edit buttons.

If we want the information to be full width, we would need to duplicate the data in a hidden div, and only show it on mobile screens.

@hursey013
Copy link
Member

Ah, gotcha, that makes sense. It looks like we may be able to gain a few more pixels by making it the full width of the parent container on mobile, guessing that will be sufficient.

**Why**: These could also overflow, or cause undesireable line breaks
@el-mapache el-mapache force-pushed the ab-adjust-account-header-columns branch from ef83ff9 to 285a1a2 Compare May 2, 2017 17:49
@el-mapache
Copy link
Contributor Author

Yep, already pushed up! 😄

Copy link
Member

@hursey013 hursey013 left a comment

Choose a reason for hiding this comment

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

lgtm 👍

@el-mapache el-mapache merged commit 7a10bf5 into master May 3, 2017
@el-mapache el-mapache deleted the ab-adjust-account-header-columns branch May 3, 2017 14:47
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.

2 participants